Page MenuHomePhabricator

khchen (Kuan Hsu Chen (Zakk))
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 23 2013, 8:22 AM (326 w, 2 d)

Recent Activity

Thu, Jan 16

khchen updated the diff for D70837: [RISCV] Support ABI checking with per function target-features.

I fixed bug here, but I'm not sure this small fixed should be another commit or not.
@lenary, do you have any suggestion?

Thu, Jan 16, 7:03 PM · Restricted Project
khchen reopened D70837: [RISCV] Support ABI checking with per function target-features.
Thu, Jan 16, 7:03 PM · Restricted Project
khchen requested review of D70837: [RISCV] Support ABI checking with per function target-features.
Thu, Jan 16, 7:03 PM · Restricted Project
khchen added a comment to D70837: [RISCV] Support ABI checking with per function target-features.

@khchen We have a load of EXPENSIVE_CHECKS RISCV MC + asm failures, a lot of which appear to be related to these changes to RISCVAsmParser please can you take a look?

First appeared:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/21599/

Current:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/21630/steps/test-check-all/logs/stdio

Thu, Jan 16, 6:15 PM · Restricted Project

Wed, Jan 15

khchen added a comment to D72755: [RISCV] Pass target-abi via module flag metadata.

Please can you also add code for ensuring that the target-abi module flag matches the -target-abi command line flag in llvm?

Wed, Jan 15, 7:07 AM · Restricted Project
khchen created D72768: [RISCV] Check the target-abi module flag matches the -target-abi option.
Wed, Jan 15, 7:00 AM · Restricted Project
khchen created D72755: [RISCV] Pass target-abi via module flag metadata.
Wed, Jan 15, 2:01 AM · Restricted Project

Tue, Jan 14

khchen added inline comments to D70837: [RISCV] Support ABI checking with per function target-features.
Tue, Jan 14, 7:51 PM · Restricted Project
khchen added a comment to D72624: [WIP] TargetMachine Hook for Module Metadata.

I think putting the resetTargetDefaultOptions after instance of TargetMachine is too late.
for example:
ppc and mips compute the TargetABI in XXXXTargetMachine constructor. In addition , mips compute the DataLayout with target ABI in TargetMachine constructor.

Tue, Jan 14, 5:07 AM · Restricted Project, Restricted Project, Restricted Project
khchen added a comment to D70837: [RISCV] Support ABI checking with per function target-features.

Hi @lenary,
This patch is necessary for enabling LTO
backend does this mismatch checking before getSubtargetImpl which reads the target-feature info from IR function attribute, and it will reset the target-abi if mismatching happen.
because I don't want to break this mismatch checking feature, so I move this checking later than getSubtargetImpl execution.

Tue, Jan 14, 1:13 AM · Restricted Project
khchen updated the summary of D70837: [RISCV] Support ABI checking with per function target-features.
Tue, Jan 14, 12:26 AM · Restricted Project

Mon, Jan 13

khchen added a comment to D72624: [WIP] TargetMachine Hook for Module Metadata.

I think putting the resetTargetDefaultOptions after instance of TargetMachine is too late.
for example:
ppc and mips compute the TargetABI in XXXXTargetMachine constructor. In addition , mips compute the DataLayout with target ABI in TargetMachine constructor.

Mon, Jan 13, 7:27 PM · Restricted Project, Restricted Project, Restricted Project

Sun, Jan 12

khchen updated subscribers of D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata.

Hi @efriedma
Could you please guide me and review this PoC?
or take a look at this [[ take a look at | maillist ]] thread?
Thank you!

Sun, Jan 12, 5:46 PM · Restricted Project, Restricted Project

Thu, Jan 9

khchen added inline comments to D71124: [RISCV] support clang driver to select cpu.
Thu, Jan 9, 10:10 PM · Restricted Project
khchen updated the diff for D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata.

remote LTO related code.
this PoC include D70837 patch for generate correct code.

Thu, Jan 9, 9:45 AM · Restricted Project, Restricted Project
khchen updated the diff for D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata.
Thu, Jan 9, 8:02 AM · Restricted Project, Restricted Project

Mon, Jan 6

khchen added a comment to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

I'd like to discuss what is good way to pass target-abi in maillist after I had a try to encoding ABI info into LLVM IR via module flag.
any suggestions are welcome, thanks.

Mon, Jan 6, 1:21 AM · Restricted Project

Sun, Jan 5

khchen created D72246: [PoC][RISCV][LTO] Pass target-abi via module flag metadata (solution 2 ).
Sun, Jan 5, 10:04 PM · Restricted Project
khchen updated the diff for D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata.
Sun, Jan 5, 9:55 PM · Restricted Project, Restricted Project
khchen created D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata.
Sun, Jan 5, 9:46 PM · Restricted Project, Restricted Project

Dec 19 2019

khchen planned changes to D71124: [RISCV] support clang driver to select cpu.

The problem is how -mcpu interact with explicitly specified -march (or target features).

Dec 19 2019, 10:34 PM · Restricted Project
khchen added a comment to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

But in RISCV clang emits the same IR for different ABI (-mabi),

This is not true. For simple cases, it does, yes, but there are some weird edge cases for functions with many arguments.

Dec 19 2019, 6:43 PM · Restricted Project

Dec 17 2019

khchen planned changes to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

I asked @kito-cheng about the GCC LTO behavior,
GCC LTO encodes the ABI info in elf and the behavior in above examples match to @efriedma 's responses.
So I think maybe encode ABI in module metadata flag is a good ideal.

Dec 17 2019, 11:12 PM · Restricted Project
khchen added a comment to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

Unfortunately on RISCV, the ABI info is only derived from -mabi option and the target triple does not encode ABI info (same to gcc).

How does gcc decide the default ABI for a target? Do you explicitly specify it at configure time?

Dec 17 2019, 10:12 PM · Restricted Project
khchen added inline comments to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.
Dec 17 2019, 9:07 AM · Restricted Project
khchen added a comment to D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

My primary concern with this is that I'm not sure you should be passing this information separately, as opposed to encoding it into the bitcode.

On ARM, the ABI for a file is generally derived from the target triple. For example, "arm-eabi" is soft-float, "arm-eabihf" is hard-float. This makes the intended target clear, provides some protection against accidentally using LTO with files compiled for different ABIs, and matches up well with our existing configuration flags to set a default target for a compiler.

Dec 17 2019, 9:04 AM · Restricted Project

Dec 11 2019

khchen updated the diff for D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.

update missed code..

Dec 11 2019, 11:12 PM · Restricted Project
khchen created D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO.
Dec 11 2019, 8:46 PM · Restricted Project

Dec 10 2019

khchen added inline comments to D71124: [RISCV] support clang driver to select cpu.
Dec 10 2019, 6:57 AM · Restricted Project
khchen updated the diff for D71124: [RISCV] support clang driver to select cpu.
Dec 10 2019, 6:51 AM · Restricted Project

Dec 6 2019

khchen added a comment to D68685: [RISCV] Scheduler description for Rocket Core.

BTW, I used this patch and D71124 to run the CoreMark on HiFive unleashed with -O3 -mllvm -enable-misched -mllvm -enable-post-misched -mcpu=rocket-rv64,
the result is better than -O3 -mllvm -enable-misched -mllvm -enable-post-misched and get the performance 3.5% speedup (use ld.bfd and libgcc).

Dec 6 2019, 9:07 AM · Restricted Project
khchen created D71124: [RISCV] support clang driver to select cpu.
Dec 6 2019, 8:48 AM · Restricted Project

Dec 3 2019

khchen added inline comments to D70116: [RISCV] add subtargets initialized with target feature.
Dec 3 2019, 6:18 PM · Restricted Project

Nov 29 2019

khchen updated the diff for D70837: [RISCV] Support ABI checking with per function target-features.

update summary.

Nov 29 2019, 8:29 AM · Restricted Project
khchen updated the diff for D70837: [RISCV] Support ABI checking with per function target-features.
Nov 29 2019, 6:50 AM · Restricted Project
khchen updated the diff for D70837: [RISCV] Support ABI checking with per function target-features.
Nov 29 2019, 2:43 AM · Restricted Project
khchen added a comment to D70837: [RISCV] Support ABI checking with per function target-features.
Nov 29 2019, 2:33 AM · Restricted Project

Nov 28 2019

khchen updated the summary of D70837: [RISCV] Support ABI checking with per function target-features.
Nov 28 2019, 10:13 PM · Restricted Project
khchen created D70837: [RISCV] Support ABI checking with per function target-features.
Nov 28 2019, 8:06 PM · Restricted Project
khchen updated the diff for D70116: [RISCV] add subtargets initialized with target feature.

add expected failed case, it will be fixed in a subsequent patch.

Nov 28 2019, 7:55 PM · Restricted Project

Nov 27 2019

khchen added a comment to D70116: [RISCV] add subtargets initialized with target feature.

So am I right in thinking that neither foo nor foo2 will trigger the ABI_Unknown issue, and the issue is only coming from hand-written assembly files?

Actually, you may need to specify -target-abi in the top of the subtarget-features-std-ext.ll file.

Sorry I didn't explain clearly,
The problem is caused by when I try to enable LTO and it need to pass mabi to LTO code generator (mabi is not encoded in bitcode)
like we compile below case with llc example.c -mtriple=riscv32 -mabi=ilp32f.

define float @foo(i32 %a) nounwind #0 {
  %conv = sitofp i32 %a to float
  ret float %conv
}

define float @foo2(i32 %a) nounwind #1{
  %conv = sitofp i32 %a to float
  ret float %conv
}

attributes #0 = { "target-features"="+f"}
attributes #1 = { "target-features"="+f"}

and they will show two error messages Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension.
so there is no any issue in subtarget-features-std-ext.ll without -target-abi.

when I started to fix this error, I found backend shares the ABI parsing/checking function (computeTargetABI) in codegen and MC layer when instance RISCVAsmBackend (note: IR function is not available at this point)
ref. https://github.com/llvm/llvm-project/commit/fea4957177315f83746dca90cb4c9013eb465c46

so this patch is workable, but it's not useful for LTO, not useful enough to support per function feature in backend, and then I'm not sure it's worth to upstream this patch.

This patch seems to solve the issue of working out the correct Subtarget on a per-function basis? This would suggest it is useful.

Yes. Okay, agree with you.

Presumably the next steps are to work out how to pass -target-abi through to the LTO backend via attributes? I was trying to see how other backends did this, but I'm not sure. I think the right way may be to have a feature per ABI, like ARM does here: https://github.com/llvm/llvm-project/blob/2d739f98d8a53e38bf9faa88cdb6b0c2a363fb77/clang/lib/Driver/ToolChains/Arch/ARM.cpp#L313-L319

It might be worth sending an email to llvm-dev, asking for guidance.

Nov 27 2019, 10:27 AM · Restricted Project
khchen added a comment to D70116: [RISCV] add subtargets initialized with target feature.

So am I right in thinking that neither foo nor foo2 will trigger the ABI_Unknown issue, and the issue is only coming from hand-written assembly files?

Actually, you may need to specify -target-abi in the top of the subtarget-features-std-ext.ll file.

Nov 27 2019, 4:20 AM · Restricted Project

Nov 26 2019

khchen requested review of D70116: [RISCV] add subtargets initialized with target feature.

I found RISCVABI::computeTargetABI will check ABI with target feature,
so when I apply this patch and enabling lto, the checking get failed and set ABI to ABI_Unknown.
It's not easy to fix it because target-features is per-function basis but ABI is not.

Nov 26 2019, 6:07 PM · Restricted Project

Nov 21 2019

khchen added a comment to D67508: [RISCV] support mutilib in baremetal environment.

Re-applied with test fix in https://reviews.llvm.org/rG4fccd383d571865321b4723b81c3042d2c15fd80

Nov 21 2019, 11:46 PM · Restricted Project
Zakk Chen <zakk.chen@sifive.com> committed rG4fccd383d571: [RISCV] Support mutilib in baremetal environment (authored by khchen).
[RISCV] Support mutilib in baremetal environment
Nov 21 2019, 8:06 PM
khchen updated subscribers of rGdf876a026981: [RISCV] Support mutilib in baremetal environment.

@lenary failed again,
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/483/steps/test-check-clang/logs/FAIL%3A%20Clang%3A%3Ariscv32-toolchain.c
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/483/steps/test-check-clang/logs/FAIL%3A%20Clang%3A%3Ariscv64-toolchain.c
I will setup windows platform to figure out what's going on.

Nov 21 2019, 6:17 AM
Zakk Chen <zakk.chen@sifive.com> committed rGdf876a026981: [RISCV] Support mutilib in baremetal environment (authored by khchen).
[RISCV] Support mutilib in baremetal environment
Nov 21 2019, 1:18 AM

Nov 19 2019

khchen updated subscribers of rGb6d7bbfa0043: [RISCV] Support mutilib in baremetal environment.
Nov 19 2019, 6:05 PM
Zakk Chen <zakk.chen@sifive.com> committed rGb6d7bbfa0043: [RISCV] Support mutilib in baremetal environment (authored by khchen).
[RISCV] Support mutilib in baremetal environment
Nov 19 2019, 2:19 AM
khchen closed D67508: [RISCV] support mutilib in baremetal environment.
Nov 19 2019, 2:19 AM · Restricted Project

Nov 18 2019

khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.

rebase and fix failed testcases

Nov 18 2019, 8:26 PM · Restricted Project

Nov 15 2019

khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.

rebase

Nov 15 2019, 4:47 AM · Restricted Project

Nov 14 2019

khchen planned changes to D70116: [RISCV] add subtargets initialized with target feature.

I found RISCVABI::computeTargetABI will check ABI with target feature,
so when I apply this patch and enabling lto, the checking get failed and set ABI to ABI_Unknown.
This is a problem because target-features is per-function basis but ABI is not.

Nov 14 2019, 5:58 AM · Restricted Project

Nov 12 2019

khchen added a comment to D70116: [RISCV] add subtargets initialized with target feature.

Nice, if this is the correct way to do it, then it's looking good.

I see x86 and arm both inspect the "soft-float" attribute, which seems to come from the -msoft-float clang option. Should RISC-V be using that to switch off the d and f target features?

I think in RISC-V using -mabi is a convention way for specific calling convention in gcc and clang, and more clear than -msoft-float.
but unfortunately compiling source code with -mabi=ilp32 will not generate correct value like "use-soft-float"="true" in bitcode,
so we can fix clang (?) and let backend inspect the "soft-float" attribute, or fix clang driver to pass -mabi to LTO code generator in RISC-V target.

Nov 12 2019, 7:41 AM · Restricted Project
khchen created D70116: [RISCV] add subtargets initialized with target feature.
Nov 12 2019, 3:06 AM · Restricted Project

Nov 8 2019

khchen added inline comments to D67409: [RISCV] enable LTO support, pass some options to linker..
Nov 8 2019, 3:25 AM · Restricted Project

Nov 3 2019

khchen set the repository for D67508: [RISCV] support mutilib in baremetal environment to rG LLVM Github Monorepo.
Nov 3 2019, 7:18 AM · Restricted Project
khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.
Nov 3 2019, 7:09 AM · Restricted Project

Nov 2 2019

khchen added a comment to D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults.

LGTM, thanks for the patch!

Nov 2 2019, 8:34 AM · Restricted Project

Oct 24 2019

khchen added a comment to D67508: [RISCV] support mutilib in baremetal environment.

@lenary
You patch is very useful to look up the default march, thanks!
But there is some issue if we set the default rv32 march as rv32gc.
Because the default multilib does not include rv32gc/lp32d in riscv gnu toolchain, https://github.com/riscv/riscv-gcc/blob/ed3f6ec/gcc/config/riscv/t-elf-multilib#L19
so if user does not give the default march clang will not find the library and cause linking error.
Do you think this is a clang's problem? or maybe this is just user's problem so they should config multilib to support rv32gc/lip3d2d?

Oct 24 2019, 11:32 AM · Restricted Project
khchen added inline comments to D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults.
Oct 24 2019, 11:12 AM · Restricted Project

Oct 23 2019

khchen added inline comments to D67508: [RISCV] support mutilib in baremetal environment.
Oct 23 2019, 10:11 PM · Restricted Project

Oct 15 2019

khchen added inline comments to D67508: [RISCV] support mutilib in baremetal environment.
Oct 15 2019, 10:29 AM · Restricted Project
khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.

@lenary Sorry, I don't have the plan to support MULTILIB_REUSE now.
But if it's necessary to support it, I can do it later.

Oct 15 2019, 10:29 AM · Restricted Project

Oct 14 2019

khchen added a comment to D67409: [RISCV] enable LTO support, pass some options to linker..

@khchen do you need me to commit this for you?

Oct 14 2019, 6:52 AM · Restricted Project
khchen updated the diff for D67409: [RISCV] enable LTO support, pass some options to linker..

rebase master,
https://reviews.llvm.org/D66003 changed the riscv::getRISCVTargetFeatures interface.

Oct 14 2019, 2:18 AM · Restricted Project

Sep 24 2019

khchen abandoned D67385: Pass -mcmodel to LTO plugin.
Sep 24 2019, 10:35 PM · Restricted Project
khchen added a comment to D67385: Pass -mcmodel to LTO plugin.

@tejohnson for example:

$ clang -flto a.c -c -o a.o
$ llvm-ar q a.a a.o          // archive libraries      
$ clang -flto a.a b.c -O2 -o main -mcmodel=small 

In above case user need to aware that passing -Wl,-plugin-opt=-code-model=small to plugin is necessary.
It seems to me that is inconvenience because I believe user doesn't know it.

In general, -flto should be transparent to the build process. Think about what happens in your example if you remove -flto:

$ clang a.c -c -o a.o
$ llvm-ar q a.a a.o // archive libraries
$ clang a.a b.c -O2 -o main -mcmodel=small

a.o will be a native code built with the default mcmodel and -O0.

Generally, adding -flto shouldn't change this - I would argue that it would be unexpected to apply mcmodel=small as the user chose to compile a.c with the default mcmodel. We are going more and more toward a model where info is communicated via the IR from the compile step, as the mcmodel already is. Is there a compelling reason why a.c should not get the default mcmodel in this case (without having compiled it mcmodel=small in the first place)?

Sep 24 2019, 10:38 AM · Restricted Project
khchen added a comment to D67385: Pass -mcmodel to LTO plugin.

@tejohnson for example:

$ clang -flto a.c -c -o a.o
$ llvm-ar q a.a a.o          // archive libraries      
$ clang -flto a.a b.c -O2 -o main -mcmodel=small 
Sep 24 2019, 8:57 AM · Restricted Project

Sep 23 2019

khchen added a comment to D67385: Pass -mcmodel to LTO plugin.

@tejohnson when I run the clang -flto -Wl,-plugin-opt=-help command, it shows the

--code-model=<value>                               - Choose code model
  =tiny                                            -   Tiny code model
  =small                                           -   Small code model
  =kernel                                          -   Kernel code model
  =medium                                          -   Medium code model
  =large                                           -   Large code model

so I think the clang driver need to pass this info to LTO codegen, does it make sense?

Sep 23 2019, 7:46 AM · Restricted Project

Sep 22 2019

khchen added a comment to D67508: [RISCV] support mutilib in baremetal environment.

ping

Sep 22 2019, 6:52 AM · Restricted Project
khchen added a comment to D67409: [RISCV] enable LTO support, pass some options to linker..

ping

Sep 22 2019, 6:52 AM · Restricted Project
khchen added a comment to D67385: Pass -mcmodel to LTO plugin.

ping

Sep 22 2019, 6:52 AM · Restricted Project

Sep 18 2019

khchen added inline comments to D67508: [RISCV] support mutilib in baremetal environment.
Sep 18 2019, 9:06 AM · Restricted Project
khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.
Sep 18 2019, 9:02 AM · Restricted Project
khchen updated the diff for D67508: [RISCV] support mutilib in baremetal environment.
Sep 18 2019, 8:57 AM · Restricted Project
khchen updated the diff for D67409: [RISCV] enable LTO support, pass some options to linker..

Thanks @lenary, I fixed it.

Sep 18 2019, 7:59 AM · Restricted Project

Sep 15 2019

khchen added a reviewer for D67508: [RISCV] support mutilib in baremetal environment: kito-cheng.
Sep 15 2019, 8:34 PM · Restricted Project

Sep 14 2019

khchen set the repository for D67385: Pass -mcmodel to LTO plugin to rC Clang.
Sep 14 2019, 8:45 AM · Restricted Project
khchen updated the diff for D67385: Pass -mcmodel to LTO plugin.

added a test

Sep 14 2019, 8:35 AM · Restricted Project
khchen updated the diff for D67409: [RISCV] enable LTO support, pass some options to linker..

This commit is inspired by @MaskRay's suggestion, I think maybe fix the insufficient of clang::driver:tools::AddGoldPlugin is good choose.

Sep 14 2019, 8:03 AM · Restricted Project

Sep 12 2019

khchen set the repository for D67508: [RISCV] support mutilib in baremetal environment to rC Clang.
Sep 12 2019, 5:33 PM · Restricted Project
khchen added inline comments to D67409: [RISCV] enable LTO support, pass some options to linker..
Sep 12 2019, 5:24 PM · Restricted Project
khchen created D67508: [RISCV] support mutilib in baremetal environment.
Sep 12 2019, 10:11 AM · Restricted Project
khchen updated the diff for D67409: [RISCV] enable LTO support, pass some options to linker..

@lenary Yes, you are right. add LTO support for Linux platform.

Sep 12 2019, 9:42 AM · Restricted Project

Sep 11 2019

khchen added reviewers for D67385: Pass -mcmodel to LTO plugin: mehdi_amini, dexonsmith.
Sep 11 2019, 7:49 PM · Restricted Project

Sep 10 2019

khchen created D67409: [RISCV] enable LTO support, pass some options to linker..
Sep 10 2019, 11:01 AM · Restricted Project
khchen created D67385: Pass -mcmodel to LTO plugin.
Sep 10 2019, 12:01 AM · Restricted Project

Aug 28 2014

khchen updated subscribers of rL216656: Fix: SLPVectorizer tried to move an instruction which was replaced by a vector….
Aug 28 2014, 1:08 AM
khchen updated subscribers of rL216656: Fix: SLPVectorizer tried to move an instruction which was replaced by a vector….
Aug 28 2014, 1:08 AM