[Clang][CSKY] Add the CSKY target and compiler driver

[zixuan-wu]

Summary

Add CSKY target toolchains to support csky in linux and elf environment.

It can leverage the basic universal Linux toolchain for linux environment, and only add some compile or link parameters. For elf environment, add a CSKYToolChain to support compile and link.

Also add some parameters into basic codebase of clang driver.

[rengolin]

I don't know enough about your toolchain requirements, but this looks good to me.

Please check the clang-format warnings. If you did pass clang-format, perhaps you need to upgrade to a newer one?

I won't approve just yet, to let other people review it also.

clang/test/Driver/csky-arch.c

27

I don't think this is doing what you expect it to do.

Depending on the output, you can still match all CHECK lines and not have the output you want.

To avoid issues, we usually separate tests that must pass (positive tests) like the lines above 24, from tests that must fail (negative tests) like the lines below.

[DavidSpickett]

clang/lib/Basic/Targets/CSKY.cpp

44

Any need to handle when ABI is not avbiv2 or abiv1 or will it always be one of the two?

clang/lib/Driver/ToolChains/Linux.cpp

367

Can you comment what all the .. are going to/from. I assume from some deep folder back up, down into the sysroot but an example in a comment would be good to document it.

[zixuan-wu]

clang/lib/Basic/Targets/CSKY.cpp

44

Yes. It always be one of the two.

[zixuan-wu]

clang/test/Driver/csky-arch.c

27

You mean separate into two files or two parts in one file?

[zixuan-wu]

Address comments.

[zixuan-wu]

I have updated the patch. Any more comments?

[rengolin]

clang/test/Driver/csky-arch-error.c

2

This will error out and fail the test. You need to add a not before %clang.

Check tests in clang/test/Driver/ that has not %clang as their RUN lines and do the same.

And keep all the not %clang tests separate from the %clang tests.

clang/test/Driver/csky-arch.c

29

If the RUN line fails (ex. on invalid arch name), the return value will be non-zero and the test will fail, so CHECKing for the absence of errors is no-op.

clang/test/Driver/csky-cpus.c

13

Same here, separate not tests.

[rengolin]

I'm surprised these tests are passing for you. Perhaps you're not building or running them all.

To make sure you're running your tests, you need to build both clang and llvm (-DLLVM_ENABLE_PROJECTS=clang) and run ninja/make check-all.

You can also run lit directly on each test, but I can't remember how to do that now...

[DavidSpickett]

You can also run lit directly on each test, but I can't remember how to do that now...

If you're in the build dir:

./bin/llvm-lit ../llvm-project/clang/test/...... -a

(-a gives you full output of what happens)

Some tests are a bit odd when you do this but these clang tests are usually ok.

[zixuan-wu]

In D121445#3388842, @rengolin wrote:

I'm surprised these tests are passing for you. Perhaps you're not building or running them all.

To make sure you're running your tests, you need to build both clang and llvm (-DLLVM_ENABLE_PROJECTS=clang) and run ninja/make check-all.

You can also run lit directly on each test, but I can't remember how to do that now...

I have enabled clang project building and run check-all test. Like -DLLVM_ENABLE_ASSERTIONS=On -DLLVM_ENABLE_PROJECTS="clang;llvm" -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="CSKY". So I am also surprised why those cases passed.

[zixuan-wu]

Address comments.

[rengolin]

I just applied this to a recent HEAD and got a few warnings. Please make sure there are no new warnings on changes / new files.

/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/CSKYInstrFormats.td:658:62: warning: unused template argument: R_Z_2:pattern
class R_Z_2<bits<6> sop, bits<5> pcode, string op, list<dag> pattern>
                                                             ^

In file included from /home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp:14:
/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/CSKYMachineFunctionInfo.h:21:20: warning: private field 'MF' is not used [-Wunused-private-field]
  MachineFunction &MF;
                   ^

/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/CSKYInstrInfo.cpp:480:24: warning: unused variable 'MRI' [-Wunused-variable]
  MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
                       ^

/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/Disassembler/CSKYDisassembler.cpp:169:21: warning: unused function 'DecodesFPR128RegisterClass' [-Wunused-function]
static DecodeStatus DecodesFPR128RegisterClass(MCInst &Inst, uint64_t RegNo,
                    ^
/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/Disassembler/CSKYDisassembler.cpp:199:21: warning: unused function 'DecodeGPRSPRegisterClass' [-Wunused-function]
static DecodeStatus DecodeGPRSPRegisterClass(MCInst &Inst, uint64_t RegNo,
                    ^
/home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/Disassembler/CSKYDisassembler.cpp:36:16: warning: private field 'inDataRegion' is not used [-Wunused-private-field]
  mutable bool inDataRegion = false;
               ^

etc.

There is also a test error:

******************** TEST 'Clang :: Driver/csky-toolchain.c' FAILED ********************
...
/home/rengolin/devel/llvm-project/clang/test/Driver/csky-toolchain.c:16:24: error: C-CSKY-LINUX-MULTI: expected string not found in input
// C-CSKY-LINUX-MULTI: "{{.*}}/Inputs/multilib_csky_linux_sdk/lib/gcc/csky-linux-gnuabiv2/6.3.0/../../..{{/|\\\\}}..{{/|\\\\}}csky-linux-gnuabiv2/bin{{/|\\\\}}ld"
                       ^

I'm guessing this is the path of a local sysroot you have on your machine?

If possible, try to get a new environment (container, VM, alternative machine) and make sure a clean build still passes the tests.

[zixuan-wu]

In D121445#3416518, @rengolin wrote:

There is also a test error:

******************** TEST 'Clang :: Driver/csky-toolchain.c' FAILED ********************
...
/home/rengolin/devel/llvm-project/clang/test/Driver/csky-toolchain.c:16:24: error: C-CSKY-LINUX-MULTI: expected string not found in input
// C-CSKY-LINUX-MULTI: "{{.*}}/Inputs/multilib_csky_linux_sdk/lib/gcc/csky-linux-gnuabiv2/6.3.0/../../..{{/|\\\\}}..{{/|\\\\}}csky-linux-gnuabiv2/bin{{/|\\\\}}ld"

                       ^

I'm guessing this is the path of a local sysroot you have on your machine?

If possible, try to get a new environment (container, VM, alternative machine) and make sure a clean build still passes the tests.

I have met this before because the downloading of patch will ignore empty files. You can have a check that your apply does not contain new empty files in multilib_csky_linux_sdk.

For warnings, those are not introduced by this patch. I create another NFC to remove unused variable and function.

[rengolin]

In D121445#3418195, @zixuan-wu wrote:

I have met this before because the downloading of patch will ignore empty files. You can have a check that your apply does not contain new empty files in multilib_csky_linux_sdk.

You are correct, it doesn't. :(

For warnings, those are not introduced by this patch. I create another NFC to remove unused variable and function.

Fair enough.

LGTM, thanks!

[dyung]

Hi, one of the tests you added. csky-toolchain.c seems to be failing on a Windows build bot, and from a quick look, it appears to be a path separator issue. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/216/builds/2452

[zixuan-wu]

In D121445#3431569, @dyung wrote:

Hi, one of the tests you added. csky-toolchain.c seems to be failing on a Windows build bot, and from a quick look, it appears to be a path separator issue. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/216/builds/2452

Reference to other target toolchain case, it should add unsupported directive. Fixed it at https://reviews.llvm.org/rG9906d38252d112894f304ba1b4fbdcd2cc93ab19

[MaskRay]

mips computed sysroot from GCCInstallation very early in 2013 rG08450bd55ccdc4aee4f5f73cde97e25b3c4ce5b9 and Android followed up in 2018 (D45291), but I am not sure this is the correct direction.
They are to support very special distributions. Hard coding the file hierarchy for every Linux distribution just does not scale.
I raised my concern on https://reviews.llvm.org/D134454#3824630 as well.

With https://reviews.llvm.org/D134337 (default configuration file) we should move the logic to use a default configuration file instead.

[zixuan-wu]

In D121445#3824657, @MaskRay wrote:

mips computed sysroot from GCCInstallation very early in 2013 rG08450bd55ccdc4aee4f5f73cde97e25b3c4ce5b9 and Android followed up in 2018 (D45291), but I am not sure this is the correct direction.
They are to support very special distributions. Hard coding the file hierarchy for every Linux distribution just does not scale.
I raised my concern on https://reviews.llvm.org/D134454#3824630 as well.

With https://reviews.llvm.org/D134337 (default configuration file) we should move the logic to use a default configuration file instead.

It's fine for me to use config file. I only have 2 points.

1. I agree sysroot should be separated from GCC because sysroot is not dependent to GCC and there is even not gcc when we use llvm runtime. This rule should also apply to multilib logic. Sysroot can detect multilib path individually.
2. From user view, it's not easy to add sysroot or config file path manually, so it needs good way to enable this functionality.