Skip to content

ci: changed to generate only files that exist in targets #1780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

ci: changed to generate only files that exist in targets #1780

wants to merge 1 commit into from

Conversation

sago35
Copy link
Member

@sago35 sago35 commented Apr 8, 2021

This PR will speed up make gen-device .
Reduce the number of targets for SVD parsing by using the same buildTag that can be obtained from tinygo targets.

This will allow CI to run faster.
Also, the size of the release version will be a little smaller.
(Actually, I think it will be almost the same size because of compression)

Instead, if you want to use a new chip, you need to add the buildTag to targets/*.json first, and then run make gen-device again.

after:

$ make build/gen-device-svd && time make gen-device -j 3

real    0m2.320s
user    0m11.424s
sys     0m1.329s

$ du -sk ./src/device/
27912   ./src/device/

before:

$ make build/gen-device-svd && time make gen-device -j 3                                                                                                                                                         

real    0m25.431s
user    1m26.611s
sys     0m8.942s

$ du -sk ./src/device/
207712  ./src/device/

@sago35
Copy link
Member Author

sago35 commented Apr 8, 2021

test-llvm11-go116 : make gen-device -j4

sago35
sago35 commented Apr 8, 2021
View reviewed changes
Makefile
GO111MODULE=off $(GO) fmt ./src/device/nxp

gen-device-sam: build/gen-device-svd
./build/gen-device-svd -source=https://github.com/posborne/cmsis-svd/tree/master/data/Atmel lib/cmsis-svd/data/Atmel/ src/device/sam/
./build/gen-device-svd -source=https://github.com/posborne/cmsis-svd/tree/master/data/Atmel -only-used lib/cmsis-svd/data/Atmel/ src/device/sam/
GO111MODULE=off $(GO) fmt ./src/device/sam

gen-device-sifive: build/gen-device-svd
./build/gen-device-svd -source=https://github.com/posborne/cmsis-svd/tree/master/data/SiFive-Community -interrupts=software lib/cmsis-svd/data/SiFive-Community/ src/device/sifive/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

svd in sifive could not be targeted because of different rules for file names.
However, since there is only one file, I don't see any problem.

@sago35
Copy link
Member Author

sago35 commented Apr 8, 2021

Some of the tests failed.
Even in my environment, I could not build build/gen-device-svd until I installed libclang-11-dev.

build-linux
https://app.circleci.com/pipelines/github/tinygo-org/tinygo/4699/workflows/d54e0a4d-fda8-46a1-ba93-b9f0d75c9493/jobs/26639

# Build Tinygo Release

#!/bin/bash -eo pipefail
make release deb -j3
cp -p build/release.tar.gz /tmp/tinygo.linux-amd64.tar.gz
cp -p build/release.deb    /tmp/tinygo_amd64.deb

CGO_CPPFLAGS="-I/home/circleci/project/llvm-project/llvm/include -I/home/circleci/project/llvm-build/include -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/circleci/project/llvm-build/tools/clang/include -I/home/circleci/project/llvm-project/clang/include -I/home/circleci/project/llvm-project/lld/include" CGO_CXXFLAGS="-std=c++14" CGO_LDFLAGS="/home/circleci/project/llvm-build/lib/libclang.a -L/home/circleci/project/llvm-build/lib -Wl,--start-group -lclangAnalysis -lclangARCMigrate -lclangAST -lclangASTMatchers -lclangBasic -lclangCodeGen -lclangCrossTU -lclangDriver -lclangDynamicASTMatchers -lclangEdit -lclangFormat -lclangFrontend -lclangFrontendTool -lclangHandleCXX -lclangHandleLLVM -lclangIndex -lclangLex -lclangParse -lclangRewrite -lclangRewriteFrontend -lclangSema -lclangSerialization -lclangStaticAnalyzerCheckers -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend -lclangTooling -lclangToolingASTDiff -lclangToolingCore -lclangToolingInclusions -Wl,--end-group -lstdc++ -Wl,--start-group -llldCOFF -llldCommon -llldCore -llldDriver -llldELF -llldMachO -llldMinGW -llldReaderWriter -llldWasm -llldYAML -Wl,--end-group -L/home/circleci/project/llvm-build/lib  -lLLVMOption -lLLVMMCJIT -lLLVMLTO -lLLVMPasses -lLLVMObjCARCOpts -lLLVMExtensions -lLLVMInterpreter -lLLVMExecutionEngine -lLLVMRuntimeDyld -lLLVMCoverage -lLLVMCoroutines -lLLVMipo -lLLVMInstrumentation -lLLVMVectorize -lLLVMLinker -lLLVMIRReader -lLLVMFrontendOpenMP -lLLVMAsmParser -lLLVMXtensaDisassembler -lLLVMXtensaCodeGen -lLLVMXtensaAsmParser -lLLVMXtensaDesc -lLLVMXtensaInfo -lLLVMAVRDisassembler -lLLVMAVRCodeGen -lLLVMAVRAsmParser -lLLVMAVRDesc -lLLVMAVRInfo -lLLVMWebAssemblyDisassembler -lLLVMWebAssemblyCodeGen -lLLVMWebAssemblyDesc -lLLVMWebAssemblyAsmParser -lLLVMWebAssemblyInfo -lLLVMRISCVDisassembler -lLLVMRISCVCodeGen -lLLVMRISCVAsmParser -lLLVMRISCVDesc -lLLVMRISCVUtils -lLLVMRISCVInfo -lLLVMAArch64Disassembler -lLLVMAArch64CodeGen -lLLVMAArch64AsmParser -lLLVMAArch64Desc -lLLVMAArch64Utils -lLLVMAArch64Info -lLLVMARMDisassembler -lLLVMARMCodeGen -lLLVMARMAsmParser -lLLVMARMDesc -lLLVMARMUtils -lLLVMARMInfo -lLLVMX86Disassembler -lLLVMX86AsmParser -lLLVMX86CodeGen -lLLVMCFGuard -lLLVMGlobalISel -lLLVMSelectionDAG -lLLVMAsmPrinter -lLLVMDebugInfoDWARF -lLLVMCodeGen -lLLVMTarget -lLLVMScalarOpts -lLLVMInstCombine -lLLVMAggressiveInstCombine -lLLVMTransformUtils -lLLVMBitWriter -lLLVMAnalysis -lLLVMProfileData -lLLVMObject -lLLVMTextAPI -lLLVMMCParser -lLLVMBitReader -lLLVMCore -lLLVMRemarks -lLLVMBitstreamReader -lLLVMX86Desc -lLLVMMCDisassembler -lLLVMMC -lLLVMDebugInfoCodeView -lLLVMDebugInfoMSF -lLLVMBinaryFormat -lLLVMX86Info -lLLVMSupport -lLLVMDemangle -lrt -ldl -lpthread -lm -lstdc++ " go build -buildmode exe -o build/tinygo -tags byollvm -ldflags="-X main.gitSha1=`git rev-parse --short HEAD`" .
go build -o build/gen-device-svd ./tools/gen-device-svd/
go build -o ./build/gen-device-avr ./tools/gen-device-avr/
# github.com/tinygo-org/tinygo/cgo
cgo/libclang.go:19:69: fatal error: clang-c/Index.h: No such file or directory
 #include <clang-c/Index.h> // if this fails, install libclang-10-dev
                                                                     ^
compilation terminated.
# tinygo.org/x/go-llvm
/go/pkg/mod/tinygo.org/x/go-llvm@v0.0.0-20210325115028-e7b85195e81c/analysis.go:16:93: fatal error: llvm-c/Analysis.h: No such file or directory
 #include "llvm-c/Analysis.h" // If you are getting an error here read bindings/go/README.txt
                                                                                             ^
compilation terminated.
Makefile:123: recipe for target 'build/gen-device-svd' failed
make: *** [build/gen-device-svd] Error 2
make: *** Waiting for unfinished jobs....

@aykevl
Copy link
Member

aykevl commented Apr 8, 2021

This is interesting, but I am actually considering something different: moving the generated files over to a separate repository (tinygo.org/device/stm32 etc) which can be used like a regular Go module.

I see two reasons for only building the SVD files that exist in the target files:

  • Build speed. This can be solved entirely by moving to a different repository.
  • Release tarball size. I think it's a good idea to keep the generated Go files in the release tarball (even if they're in a module), the Go release does something similar. So then there would be no savings. But I think comparing the size before and after (try make release) would give a better idea of how much it would help in practice.

What do you think?

@deadprogram
Copy link
Member

I think separate repo for the device generated code is a good idea. We can move all the generation tools into that repo as well.

@sago35
Copy link
Member Author

sago35 commented Apr 8, 2021

I also think it's a good idea to move it to a different repository.

@sago35
Copy link
Member Author

sago35 commented Apr 8, 2021

moving the generated files over to a separate repository (tinygo.org/device/stm32 etc) which can be used like a regular Go module.

How do we put it in tinygo.org/device/stm32 instead of tinygo.org/x/device/stm32? .
For example, for device/stm32, do we put it in github.com/tinygo-org/stm32?

@kenbell
Copy link
Member

kenbell commented Apr 9, 2021

I think a separate repo is a good idea. For STM32 would it include the 'patched' SVDs, or would that still be a separate repo?

@sago35
Copy link
Member Author

sago35 commented Apr 13, 2021

moving the generated files over to a separate repository (tinygo.org/device/stm32 etc) which can be used like a regular Go module.

This idea will make CI faster, so the sooner we start working on it, the better.
I don't know how to put the files in tinygo.org/device/stm32 etc, but I think what we need to do is the following

  1. Create a repository for the automatically generated device file under github.com/tinygo-org
  2. Make CI create the device file.
  3. Configure tinygo.org/device as a reference

@deadprogram
Copy link
Member

Keep in mind the package name would be tinygo.org/x/device or something like that.

@aykevl
Copy link
Member

aykevl commented Apr 13, 2021

Keep in mind the package name would be tinygo.org/x/device or something like that.

My thinking is that because the device package is so fundamental it's not really an "extra" (x) package. That's why I said tinygo.org/device. But maybe that's just an arbitrary distinction and it's better to put them all under the tinygo.org/x/ tree. I'm fine with either option.

I don't think we need to generate the files in CI. The files are not very frequently updated and I think manually updating is good enough, similar to the stm32-svd repository. Doing it in CI may be complicated.

(Historical sidenote: a long time ago I checked in all generated files for AVR, but later reverted this. This is why https://github.com/tinygo-org/tinygo/graphs/code-frequency has this huge spike. Doing this in a separate repo avoids this).

@deadprogram
Copy link
Member

deadprogram commented Apr 13, 2021 •
edited
Loading

The x has to do with how the the static redirect for the "vanity domain" is setup. Copied the pattern from golang.org/x/crypto/ssh etc.

@sago35 sago35 mentioned this pull request Apr 24, 2021
Draft
@deadprogram
Copy link
Member

Closing due to age. Thanks to everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sago35 @aykevl @deadprogram @kenbell