[llvm] [RISCV] Bugfix for FCLASS incorrect regbankselect (PR #118021)
Luke Quinn via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 29 09:14:38 PST 2024
================
@@ -1,10 +1,10 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple=riscv32 -mattr=+d,+zfh -run-pass=regbankselect \
-# RUN: -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
-# RUN: -o - | FileCheck %s --check-prefixes=CHECK
-# RUN: llc -mtriple=riscv64 -mattr=+d,+zfh -run-pass=regbankselect \
-# RUN: -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
-# RUN: -o - | FileCheck %s --check-prefixes=CHECK
+# RUN: sed 's/XLen/32/g' %s | llc -mtriple=riscv32 -mattr=+d,+zfh -run-pass=regbankselect \
+# RUN: -disable-gisel-legality-check -simplify-mir -verify-machineinstrs -x mir \
+# RUN: -o - | FileCheck %s --check-prefixes=CHECK-RV32
+# RUN: sed 's/XLen/64/g' %s | llc -mtriple=riscv64 -mattr=+d,+zfh -run-pass=regbankselect \
+# RUN: -disable-gisel-legality-check -simplify-mir -verify-machineinstrs -x mir \
+# RUN: -o - | FileCheck %s --check-prefixes=CHECK-RV64
----------------
lquinn2015 wrote:
I would some what agree that spliting the test up would be more reasonable but that only case in the the new fp-load-store.mir test required that sed replacement was the one I added so it seemed sad to force duplication for just 1 part of the test case. On the other hand if we keep the tests conjoined it makes sure RV32 and RV64 cannot diverge on register select for these simple cases which is probably better imo. @topperc asked me to keep them together maybe he had a different reason.
You are correct that the update__mir_test_checks did not (past tense :)) handle preprocessor commands however I added that to the python script using the same infra that the update_llc_test_checks script uses so the two of them converged a little bit which I think is a plus. I feel as though this is better as a MIR test because its directly testing the register select behavior that was changed. IR would of course test that in a full sweep but it would also be indirectly testing a bunch of different behaviors that might change in the future. Let me know what you think you guys are the bosses!
https://github.com/llvm/llvm-project/pull/118021
More information about the llvm-commits
mailing list