Page MenuHomePhabricator

[RISCV] Pre-RA expand pseudos pass
ClosedPublic

Authored by luismarques on Apr 6 2022, 4:02 PM.

Details

Summary

As discussed before, we aren't optimizing the LLA+Load/Stores (i.e. AUIPC+ADDI+Load/Stores) instruction sequences that we get with medany because:

  • the pseudo instruction expansion pass runs late (in premit2, i.e. post-ra);
  • the pass to merge the offset of address calculations into the offset of load/stores runs pre-ra, in SSA form.

Ideally, we want to expand the LLA pseudo instruction earlier and extend the offset folding pass to handle the AUIPC case. This patch implements that earlier expansion.

Just doing the same expansion we were doing but earlier, with virtual registers, runs into problems. It's easy for optimizations to separate the AUIPC instruction from the label of the BB that should point to the AUIPC.

Earlier passes don't know about LabelMustBeEmitted. Originally I solved that by making that flag also imply AddressTaken. But creating BBs earlier was messing with various optimizations, so I ended up going for an implementation based on createNamedTempSymbol + setPreInstrSymbol.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
luismarques requested review of this revision.Apr 6 2022, 4:02 PM
HsiangKai added inline comments.
llvm/include/llvm/CodeGen/MachineInstr.h
1210

Is it enough to only add checking of getPreInstrSymbol()?

HsiangKai added inline comments.Apr 15 2022, 1:26 AM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
298

redundant?

luismarques added inline comments.Apr 15 2022, 1:47 AM
llvm/include/llvm/CodeGen/MachineInstr.h
1210

What do you mean, enough for what? (This patch also checks getPostInstrSymbol()).
In my testing that was enough to ensure that the label always pointed to the correct upper instruction, but feedback about this is welcome.

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
298

Thanks. I'll remove it in the next revision.

HsiangKai added inline comments.Apr 15 2022, 2:29 AM
llvm/include/llvm/CodeGen/MachineInstr.h
1210

Just curious why checking getPostInstrSymbol() here. There is no setPostInstrSymbol() in this patch.

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
84–85

Do we still need to handle PseudoLLA in RISCVExpandPseudo?

luismarques marked 2 inline comments as done.
luismarques added inline comments.
llvm/include/llvm/CodeGen/MachineInstr.h
1210

It was just for regularity. My reasoning was that if some_label: INSTR; is a position then INSTR; some_label: also is (assuming the label is attached to the INSTR), so this change made sense for both. But it's not necessary for *this* particular patch. Thanks for checking that oddity :)

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
84–85

Possibly not. I'll have to more carefully check if there aren't passes that can reintroduce those instructions. I wonder if it just isn't safer to leave it for now, anyway? In any case, if this patch is accepted and proves stable then it might make sense to eventually move more/all of these expansions to Pre-RA, as that unlocks various optimizations. Sam Elliot had explored that possibility some time ago but he had run into the issue of the instructions and the labels getting out of sync. That seems solved now with my isPosition change.

HsiangKai added inline comments.Apr 15 2022, 7:55 AM
llvm/include/llvm/CodeGen/MachineInstr.h
1210

To change the condition of isPosition() will have side effects in other passes. One example is instruction scheduling. isPosition() is one of the condition to decide region boundaries for instruction scheduling. Refer to https://github.com/llvm/llvm-project/blob/be5c15c7aee1ef4964c88031555ed6b0f59ebc23/llvm/lib/CodeGen/TargetInstrInfo.cpp#L1025

llvm/test/CodeGen/RISCV/codemodel-lowering.ll
136–137

Here is an example that to change isPosition() will change instruction scheduling behavior. After the patch, auipc will become the boundary of scheduling regions. fmv.w.x will become the only instruction in its own region. If there are multiple latencies in load-to-use in the microarchitecture, fmv.w.x will not be able to fill the latencies after flw. It will have negative impact on the performance.

StephenFan added inline comments.Apr 15 2022, 9:56 AM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
351

Using createTempSymbol to create a symbol will cause the object code's string table to have an empty name for the symbol. Maybe you could use the MF.getPICBaseSymbol() function to create the PIC base symbol. It can create the function local symbol by using the unique function id.

StephenFan added a comment.EditedApr 16 2022, 10:10 AM
This comment has been deleted.
llvm/include/llvm/CodeGen/MachineInstr.h
1210

What do you mean, enough for what? (This patch also checks getPostInstrSymbol()).
In my testing that was enough to ensure that the label always pointed to the correct upper instruction, but feedback about this is welcome.

I'm curious about by setPreInstrSymbol, the compiler still can't ensure that the label always pointed
to the correct upper instruction? I think after setPreInstrSymbol, the AsmPrinter will always emit the symbol label before this instruction.

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
355

Because we have set a pre-instr symbol and this symbol is unique, I think we need to make the AUIPC instruction not duplicable. If auipc instruction were duplicated, the unique symbol would also have multiple definitions. It is in conflict with the symbol is unique.

luismarques marked 2 inline comments as done.
luismarques edited the summary of this revision. (Show Details)

Don't mess with isPosition. Create named temp labels. Remove cruft.

luismarques marked 3 inline comments as done.Apr 18 2022, 4:40 PM
luismarques added inline comments.
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
351

I'll look into this more carefully later. I thought (but didn't verify) that createNamedTempSymbol would emit the symbol like you wanted, even if that symbol wasn't then function-local. But then I looked at object files built with the original patch and they seemed already have the .LPCRelHi0 etc in .symtab. So I'm not sure I follow your point. As I said, I need to look into this more carefully.

355

That makes sense. It wasn't immediately obvious how to dynamically set the not duplicable flag per instruction instance, so I'll handle this one later. There seemed to be fewer passes that checked the isNotDuplicable property than I would expect, though.

luismarques added inline comments.Apr 18 2022, 4:54 PM
llvm/include/llvm/CodeGen/MachineInstr.h
1210

I'm curious about by setPreInstrSymbol, the compiler still can't ensure that the label always pointed
to the correct upper instruction? I think after setPreInstrSymbol, the AsmPrinter will always emit the symbol label before this instruction.

Indeed. There was an issue where the label for the upper instruction was being removed, and it was seemingly caused by the branch folder pass. I think I misinterpreted what was going on there, possibly because I was originally using a different implementation and got confused. When I looked again into that it seemed that the invalid transformation made sense if two AUIPCs were actually the same. They were, except for the pre and post-instruction symbols. Sure enough, extending the MI comparison to check for the pre- and post-instruction symbols solved the issue without messing with the isPosition predicate. Thanks.

jrtc27 added inline comments.Apr 19 2022, 10:21 AM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
348

As a quick non-technical comment, if we're going to give names to these symbols, can we please use .Lpcrel_hiN to match what you get from expanding the pseudos in RISCVAsmParser rather than a different spelling of the same thing?

  • Although I haven't run into that problem, I think it's still possible for an ADDI to become out of range of the AUIPC it refers to, if other passes move them too far apart. For branches we have the branch relaxation pass to solve essentially the same problem. Are we going to need a similar pass for AUIPC/ADDI? Or can we prevent this from happening?

This doesn't make sense to me? The immediate for the ADDI is unused in the .o file, and in the linked file it's just the low 12 bits of the 32-bit offset of the symbol from the AUIPC. Where you put the ADDI never matters, so long as it's in the same input section as the AUIPC its relocation refers to.

llvm/lib/CodeGen/MachineInstr.cpp
634

This is overly strict in the specific case being addressed by this patch:

1: auipc a0, %pcrel_hi(foo)
   lw a1, %pcrel_lo(1b)(a0)
   addi a1, a1, 1
2: auipc a0, %pcrel_hi(foo)
   sw a1, %pcrel_lo(2b)(a0)

can legally be optimised to

1: auipc a0, %pcrel_hi(foo)
   lw a1, %pcrel_lo(1b)(a0)
   addi a1, a1, 1
   sw a1, %pcrel_lo(1b)(a0)

This doesn't make sense to me?

Right, that was silly, I was confused. I'll remove it from the summary.

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
348

Yes, I'll update it in the next revision.

luismarques edited the summary of this revision. (Show Details)Apr 19 2022, 11:14 AM
luismarques added inline comments.Apr 19 2022, 12:17 PM
llvm/lib/CodeGen/MachineInstr.cpp
634

That is a generic MachineInstr comparison function. We don't make use of it to reason about these address generation instructions sequences, it's here only to tighten the checks for other passes (namely the branch folder pass, possibly others) that would break once we start using pre-instruction symbols. So I guess it's fine as it? It's also not immediately obvious to me in which ways we could be more clever about the comparison and still keep it conservative enough to be target and "context" independent.

craig.topper added inline comments.Jun 23 2022, 12:00 PM
llvm/test/CodeGen/RISCV/O3-pipeline.ll
107

Can we say that the new pass preserves the CFG? Will that prevent this change?

  • Rebase
  • Apply suggestion to rename label name
  • Set AnalysisUsage CFG preserved

I think the only big thing missing is that the instructions with labels must be marked as non-copyable.

luismarques marked 2 inline comments as done.Jun 24 2022, 3:50 AM

Make instructions with pre- and post-instructions symbols not duplicable

luismarques marked an inline comment as done.Jul 7 2022, 7:20 AM
luismarques added inline comments.
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
351

Using createTempSymbol to create a symbol will cause the object code's string table to have an empty name for the symbol.

That doesn't seem to be the case.

This revision is now accepted and ready to land.Wed, Jul 27, 10:30 AM
jrtc27 requested changes to this revision.Wed, Jul 27, 10:34 AM

LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.

This revision now requires changes to proceed.Wed, Jul 27, 10:34 AM

(though I would prefer _all_ LA-like instructions be expanded at the same point, not just the ones where you can fold the immediate in; both the -fPIC LA and the TLS ones would ideally be expanded at the same time as LLA)

  • Move the other LA-like instructions to the pre-ra expansion pass
  • Merge back the mir-target-flags.ll test. The splitting done in previous versions of the patch is no longer needed as the pre-instruction symbol printing now seems to be implemented.

LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.

Does the compiler ever emit an LA-as-LLA? The branch in RISCVExpandPseudoInst is untested according to the coverage report https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp.html#L217

craig.topper added inline comments.Fri, Jul 29, 4:01 PM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
375

Should preserve the memoperand the call may have had if this is a load? The original expansion was late in the pipe so probably didn't matter. That may be different now that its earlier.

LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.

Does the compiler ever emit an LA-as-LLA? The branch in RISCVExpandPseudoInst is untested according to the coverage report https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp.html#L217

LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.

Does the compiler ever emit an LA-as-LLA? The branch in RISCVExpandPseudoInst is untested according to the coverage report https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp.html#L217

I think that branch was already dead when it was originally added: https://reviews.llvm.org/D55303
RISCV::PseudoLA was only emitted inside an if (isPositionIndependent()), and the dead branch is the negation of that condition.
Should we remove the dead branch in a separate patch?

  • Remove the unused RISCVExpandPseudo::expandAuipcInstPair
  • Delete the dead branch from the RISCVPreRAExpandPseudo::expandAuipcInstPair
  • Try to address the feedback regarding preserving the memoperands
  • Update mir-target-flags.ll due to reorderings caused by the memoperands
luismarques added inline comments.Sat, Jul 30, 3:54 PM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
375

Does the change below address this feedback?

llvm/test/CodeGen/RISCV/mir-target-flags.ll
26–27

I tried to add flags to llc to disable any scheduling reorderings but failed to preserve the original order. Any suggestions?

  • Add missing initialization/registration of the new pass
  • Update the mir-target-flags.ll to make it insensitive to the machine-scheduler reorderings that happened once the patch started to preserve the memoperands.
craig.topper accepted this revision.Sun, Jul 31, 2:08 PM

LGTM

llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
380

nit: drop curly braces

This revision was not accepted when it landed; it landed in state Needs Review.Sun, Jul 31, 2:20 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Hi I think this patch led to the following ICEs seen on our windows builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8807088318544910865/overview):

96/151] Building C object CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj
FAILED: CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj 
C:\b\s\w\ir\x\w\staging\llvm_build\.\bin\clang.exe -DVISIBILITY_HIDDEN  --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include   -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj   -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
Unhandled encodeInstruction length!
UNREACHABLE executed at C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\MCTargetDesc\RISCVMCCodeEmitter.cpp:209!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: C:\\b\\s\\w\\ir\\x\\w\\staging\\llvm_build\\.\\bin\\clang.exe -DVISIBILITY_HIDDEN --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c'.
4.	Running pass 'RISCV Assembly Printer' on function '@__floatdidf'
Exception Code: 0xC000001D
 #0 0x00007ff6a2ebbcb6 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x6dbcb6)
 #1 0x00007ff6a6d19bcb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4539bcb)
 #2 0x00007ff6a6d0f0d0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x452f0d0)
 #3 0x00007ff6a2e7e175 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69e175)
 #4 0x00007ff6a374a155 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0xf6a155)
 #5 0x00007ff6a2c350aa (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4550aa)
 #6 0x00007ff6a3b6669c (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138669c)
 #7 0x00007ff6a29197c8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1397c8)
 #8 0x00007ff6a30cd61e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8ed61e)
 #9 0x00007ff6a2918d43 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138d43)
#10 0x00007ff6a31301ca (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x9501ca)
#11 0x00007ff6a2b000f8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3200f8)
#12 0x00007ff6a2b070cc (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3270cc)
#13 0x00007ff6a2b00829 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x320829)
#14 0x00007ff6a3d77231 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1597231)
#15 0x00007ff6a4176f58 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1996f58)
#16 0x00007ff6a58a9e4e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x30c9e4e)
#17 0x00007ff6a40d5484 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x18f5484)
#18 0x00007ff6a302fc23 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x84fc23)
#19 0x00007ff6a30c4dc4 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8e4dc4)
#20 0x00007ff6a27e7abe (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x7abe)
#21 0x00007ff6a27e4848 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4848)
#22 0x00007ff6a3e87636 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a7636)
#23 0x00007ff6a2e7ca22 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69ca22)
#24 0x00007ff6a3e872cf (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a72cf)
#25 0x00007ff6a2fed54a (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d54a)
#26 0x00007ff6a2fed7af (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d7af)
#27 0x00007ff6a3006cfb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x826cfb)
#28 0x00007ff6a27e418f (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x418f)
#29 0x00007ff6a6cfadf0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x451adf0)
#30 0x00007ff90b9f7974 (C:\Windows\System32\KERNEL32.DLL+0x17974)
#31 0x00007ff90e73a2f1 (C:\Windows\SYSTEM32\ntdll.dll+0x5a2f1)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Fuchsia clang version 16.0.0 (https://llvm.googlesource.com/llvm-project 260a64106854986a981e49ed87ee740460a23eb5)
Target: riscv64-unknown-fuchsia
Thread model: posix
InstalledDir: C:\b\s\w\ir\x\w\staging\llvm_build\.\bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.c
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.sh

clang: note: diagnostic msg: 



********************

Would you be able to send out a fix or revert?

Hi I think this patch led to the following ICEs seen on our windows builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8807088318544910865/overview):

96/151] Building C object CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj
FAILED: CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj 
C:\b\s\w\ir\x\w\staging\llvm_build\.\bin\clang.exe -DVISIBILITY_HIDDEN  --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include   -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj   -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
Unhandled encodeInstruction length!
UNREACHABLE executed at C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\MCTargetDesc\RISCVMCCodeEmitter.cpp:209!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: C:\\b\\s\\w\\ir\\x\\w\\staging\\llvm_build\\.\\bin\\clang.exe -DVISIBILITY_HIDDEN --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c'.
4.	Running pass 'RISCV Assembly Printer' on function '@__floatdidf'
Exception Code: 0xC000001D
 #0 0x00007ff6a2ebbcb6 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x6dbcb6)
 #1 0x00007ff6a6d19bcb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4539bcb)
 #2 0x00007ff6a6d0f0d0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x452f0d0)
 #3 0x00007ff6a2e7e175 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69e175)
 #4 0x00007ff6a374a155 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0xf6a155)
 #5 0x00007ff6a2c350aa (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4550aa)
 #6 0x00007ff6a3b6669c (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138669c)
 #7 0x00007ff6a29197c8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1397c8)
 #8 0x00007ff6a30cd61e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8ed61e)
 #9 0x00007ff6a2918d43 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138d43)
#10 0x00007ff6a31301ca (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x9501ca)
#11 0x00007ff6a2b000f8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3200f8)
#12 0x00007ff6a2b070cc (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3270cc)
#13 0x00007ff6a2b00829 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x320829)
#14 0x00007ff6a3d77231 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1597231)
#15 0x00007ff6a4176f58 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1996f58)
#16 0x00007ff6a58a9e4e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x30c9e4e)
#17 0x00007ff6a40d5484 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x18f5484)
#18 0x00007ff6a302fc23 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x84fc23)
#19 0x00007ff6a30c4dc4 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8e4dc4)
#20 0x00007ff6a27e7abe (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x7abe)
#21 0x00007ff6a27e4848 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4848)
#22 0x00007ff6a3e87636 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a7636)
#23 0x00007ff6a2e7ca22 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69ca22)
#24 0x00007ff6a3e872cf (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a72cf)
#25 0x00007ff6a2fed54a (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d54a)
#26 0x00007ff6a2fed7af (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d7af)
#27 0x00007ff6a3006cfb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x826cfb)
#28 0x00007ff6a27e418f (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x418f)
#29 0x00007ff6a6cfadf0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x451adf0)
#30 0x00007ff90b9f7974 (C:\Windows\System32\KERNEL32.DLL+0x17974)
#31 0x00007ff90e73a2f1 (C:\Windows\SYSTEM32\ntdll.dll+0x5a2f1)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Fuchsia clang version 16.0.0 (https://llvm.googlesource.com/llvm-project 260a64106854986a981e49ed87ee740460a23eb5)
Target: riscv64-unknown-fuchsia
Thread model: posix
InstalledDir: C:\b\s\w\ir\x\w\staging\llvm_build\.\bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.c
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.sh

clang: note: diagnostic msg: 



********************

Would you be able to send out a fix or revert?

Are you able to provide the preprocessed source file floatdidf-9493d4.c from the crash dump?

Hi I think this patch led to the following ICEs seen on our windows builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8807088318544910865/overview):

96/151] Building C object CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj
FAILED: CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj 
C:\b\s\w\ir\x\w\staging\llvm_build\.\bin\clang.exe -DVISIBILITY_HIDDEN  --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include   -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj   -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
Unhandled encodeInstruction length!
UNREACHABLE executed at C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\MCTargetDesc\RISCVMCCodeEmitter.cpp:209!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: C:\\b\\s\\w\\ir\\x\\w\\staging\\llvm_build\\.\\bin\\clang.exe -DVISIBILITY_HIDDEN --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c'.
4.	Running pass 'RISCV Assembly Printer' on function '@__floatdidf'
Exception Code: 0xC000001D
 #0 0x00007ff6a2ebbcb6 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x6dbcb6)
 #1 0x00007ff6a6d19bcb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4539bcb)
 #2 0x00007ff6a6d0f0d0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x452f0d0)
 #3 0x00007ff6a2e7e175 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69e175)
 #4 0x00007ff6a374a155 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0xf6a155)
 #5 0x00007ff6a2c350aa (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4550aa)
 #6 0x00007ff6a3b6669c (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138669c)
 #7 0x00007ff6a29197c8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1397c8)
 #8 0x00007ff6a30cd61e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8ed61e)
 #9 0x00007ff6a2918d43 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138d43)
#10 0x00007ff6a31301ca (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x9501ca)
#11 0x00007ff6a2b000f8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3200f8)
#12 0x00007ff6a2b070cc (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3270cc)
#13 0x00007ff6a2b00829 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x320829)
#14 0x00007ff6a3d77231 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1597231)
#15 0x00007ff6a4176f58 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1996f58)
#16 0x00007ff6a58a9e4e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x30c9e4e)
#17 0x00007ff6a40d5484 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x18f5484)
#18 0x00007ff6a302fc23 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x84fc23)
#19 0x00007ff6a30c4dc4 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8e4dc4)
#20 0x00007ff6a27e7abe (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x7abe)
#21 0x00007ff6a27e4848 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4848)
#22 0x00007ff6a3e87636 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a7636)
#23 0x00007ff6a2e7ca22 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69ca22)
#24 0x00007ff6a3e872cf (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a72cf)
#25 0x00007ff6a2fed54a (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d54a)
#26 0x00007ff6a2fed7af (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d7af)
#27 0x00007ff6a3006cfb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x826cfb)
#28 0x00007ff6a27e418f (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x418f)
#29 0x00007ff6a6cfadf0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x451adf0)
#30 0x00007ff90b9f7974 (C:\Windows\System32\KERNEL32.DLL+0x17974)
#31 0x00007ff90e73a2f1 (C:\Windows\SYSTEM32\ntdll.dll+0x5a2f1)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Fuchsia clang version 16.0.0 (https://llvm.googlesource.com/llvm-project 260a64106854986a981e49ed87ee740460a23eb5)
Target: riscv64-unknown-fuchsia
Thread model: posix
InstalledDir: C:\b\s\w\ir\x\w\staging\llvm_build\.\bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.c
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.sh

clang: note: diagnostic msg: 



********************

Would you be able to send out a fix or revert?

Are you able to provide the preprocessed source file floatdidf-9493d4.c from the crash dump?

At the moment, no. I can try to reproduce it though that will take some time.

Hi I think this patch led to the following ICEs seen on our windows builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8807088318544910865/overview):

96/151] Building C object CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj
FAILED: CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj 
C:\b\s\w\ir\x\w\staging\llvm_build\.\bin\clang.exe -DVISIBILITY_HIDDEN  --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include   -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj   -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
Unhandled encodeInstruction length!
UNREACHABLE executed at C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\MCTargetDesc\RISCVMCCodeEmitter.cpp:209!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: C:\\b\\s\\w\\ir\\x\\w\\staging\\llvm_build\\.\\bin\\clang.exe -DVISIBILITY_HIDDEN --target=riscv64-unknown-fuchsia -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/sync/include -IC:/b/s/w/ir/x/w/cipd/sdk/pkg/fdio/include -DCOMPILER_RT_HAS_FLOAT16 -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -o CMakeFiles/clang_rt.builtins-riscv64.dir/floatdidf.c.obj -c C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'C:/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/floatdidf.c'.
4.	Running pass 'RISCV Assembly Printer' on function '@__floatdidf'
Exception Code: 0xC000001D
 #0 0x00007ff6a2ebbcb6 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x6dbcb6)
 #1 0x00007ff6a6d19bcb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4539bcb)
 #2 0x00007ff6a6d0f0d0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x452f0d0)
 #3 0x00007ff6a2e7e175 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69e175)
 #4 0x00007ff6a374a155 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0xf6a155)
 #5 0x00007ff6a2c350aa (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4550aa)
 #6 0x00007ff6a3b6669c (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138669c)
 #7 0x00007ff6a29197c8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1397c8)
 #8 0x00007ff6a30cd61e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8ed61e)
 #9 0x00007ff6a2918d43 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x138d43)
#10 0x00007ff6a31301ca (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x9501ca)
#11 0x00007ff6a2b000f8 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3200f8)
#12 0x00007ff6a2b070cc (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x3270cc)
#13 0x00007ff6a2b00829 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x320829)
#14 0x00007ff6a3d77231 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1597231)
#15 0x00007ff6a4176f58 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x1996f58)
#16 0x00007ff6a58a9e4e (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x30c9e4e)
#17 0x00007ff6a40d5484 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x18f5484)
#18 0x00007ff6a302fc23 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x84fc23)
#19 0x00007ff6a30c4dc4 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x8e4dc4)
#20 0x00007ff6a27e7abe (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x7abe)
#21 0x00007ff6a27e4848 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x4848)
#22 0x00007ff6a3e87636 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a7636)
#23 0x00007ff6a2e7ca22 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x69ca22)
#24 0x00007ff6a3e872cf (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x16a72cf)
#25 0x00007ff6a2fed54a (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d54a)
#26 0x00007ff6a2fed7af (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x80d7af)
#27 0x00007ff6a3006cfb (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x826cfb)
#28 0x00007ff6a27e418f (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x418f)
#29 0x00007ff6a6cfadf0 (C:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe+0x451adf0)
#30 0x00007ff90b9f7974 (C:\Windows\System32\KERNEL32.DLL+0x17974)
#31 0x00007ff90e73a2f1 (C:\Windows\SYSTEM32\ntdll.dll+0x5a2f1)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Fuchsia clang version 16.0.0 (https://llvm.googlesource.com/llvm-project 260a64106854986a981e49ed87ee740460a23eb5)
Target: riscv64-unknown-fuchsia
Thread model: posix
InstalledDir: C:\b\s\w\ir\x\w\staging\llvm_build\.\bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.c
clang: note: diagnostic msg: C:\b\s\w\ir\x\t\floatdidf-9493d4.sh

clang: note: diagnostic msg: 



********************

Would you be able to send out a fix or revert?

Are you able to provide the preprocessed source file floatdidf-9493d4.c from the crash dump?

At the moment, no. I can try to reproduce it though that will take some time.

Nevermind, that reproduced quite easily with ./bin/clang --target=riscv64-unknown-fuchsia ../compiler-rt/lib/builtins/floatdidf.c. I'll look at it right after I finish lunch.

Looks like we aren't running the pass at -O0. I didn't think about the fact that only O3-pipeline.ll and not O0-pipeline.ll was updated.

@leonardchan I pushed e07a8155f5168fdaff9346152d7805a47cb49405 to enable for -O0. Hopefully that resolves the crash.

@leonardchan I pushed e07a8155f5168fdaff9346152d7805a47cb49405 to enable for -O0. Hopefully that resolves the crash.

Thanks Craig. Facepalm moment for me...