-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[RISCV] Fix CFI Multiple Locations Test #168772
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.
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
Conversation
|
@llvm/pr-subscribers-backend-risc-v Author: Sam Elliott (lenary) ChangesFull diff: https://github.com/llvm/llvm-project/pull/168772.diff 1 Files Affected:
diff --git a/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir b/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir
index 08544a95dedb7..d2e5a8208cecf 100644
--- a/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir
+++ b/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir
@@ -1,4 +1,4 @@
-# RUN: llc %s -mtriple=riscv64 \
+# RUN: llc -x mir < %s -mtriple=riscv64 \
# RUN: -run-pass=cfi-instr-inserter \
# RUN: -riscv-enable-cfi-instr-inserter=true
# UNSUPPORTED: target={{.*}}
|
|
This addresses the issue in #164479 (comment) |
|
Is this just making it output to stdout because using stdin defaults to using stdout? |
|
I'm working on a more complete fix to make this test work again. |
Looks like we have a test for the same unreachable in X86 as well, but it's got REQUIRES: asserts |
This prevents it from being optimized out in non-assert builds. Update X86 test to remove REQUIRES: asserts and check for LLVM ERROR. Add FileCheck to RISC-V test and remove UNSUPPORTED. This is the better fix for llvm#168772 and llvm#168525.
|
Sorry, I should have paid more attention. This is indeed just fixing it so it uses stdin/stdout rather than leaving a file somewhere it shouldn't. I would have directly committed this - I only opened a PR as that's what is expected of us now, but I see that's not very productive. I'll look at the better PR |
No description provided.