Details
Diff Detail
- Build Status
Buildable 27412 Build 27411: arc lint + arc unit
Event Timeline
I don't know enough about LLVM to review the code, but I just built a custom Rust compiler that uses this patch and can verify that this fixes the passed error code. The passed exception stack frame looks correct too.
Thanks a lot for fixing this @rnk!
@craig.topper, any thoughts? I think we'll want to merge this to 8.0 since Rust wants it.
- remove stray character
llvm/test/CodeGen/X86/x86-32-intrcc.ll | ||
---|---|---|
121 | I think it's a stray Vim command for "insert at end of line". :) | |
llvm/test/CodeGen/X86/x86-interrupt_cc.ll | ||
32–111 | No, it always wild-cards stack offsets, but optionally not global / RIP offsets. Do you remember having any particular reason to move to auto-generation in rL324530? I think the test only really needed to look for a few instructions, movk, and a few encodings. It doesn't make sense to me to auto-generate the tests if we card about the encoding, since the wildcarding does nothing to make the test less sensitive to unrelated changes. |
I think I autogenerated it because I added more run lines and then fixed a bug in the next commit. I'm just in the habit of using the script.
So have we been incorrectly positioning the registers in the AVX512 tests?
I think auto-generating them is fine, but we should stop checking the instruction encodings since it defeats the wildcarding. If it's valuable to check the encodings, we should have a separate, non-generated test for that.
So have we been incorrectly positioning the registers in the AVX512 tests?
So, after looking at the offsets, it uncovered a bug. I made a mistake to assume that all fixed stack objects come before the return address. I think I can tweak my X86FrameLowering.cpp change to retain the local area offset adjustment for spilled fixed stack objects (ones that would normally be part of the newly allocated frame, not arguments). I need to look into it more though.
Huh, apparently I spoke too soon, now the tests don't pass. :( Anyway, I'll move it forward.
Was that stray A at the end of the line supposed to be another sentence? its in the other test file too.