-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[update_mir_test_checks] Add missing MIFlags #150012
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
[update_mir_test_checks] Add missing MIFlags #150012
Conversation
If the update_mir_test_checks.py script is aware of MIFlags, it can produce meaningful identifiers in generated FileCheck lines. A few MIFlags that were introduced more recently have been missing from the script. Ideally, the MIFlags would be specified in a single place and automatically made known to the script to avoid this divergence, but for now adding a comment pointing to the script at the place where the MIFlags are printed seems like a reasonable trade-off. This PR only regenerates check lines for a single test as an example of the effect; other affected tests are not regenerated for now to avoid unnecessary test churn.
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-testing-tools Author: Fabian Ritter (ritter-x2a) ChangesIf the update_mir_test_checks.py script is aware of MIFlags, it can produce Ideally, the MIFlags would be specified in a single place and automatically This PR only regenerates check lines for a single test as an example of the Full diff: https://github.com/llvm/llvm-project/pull/150012.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index 7710b503facc3..bc4e2999b750a 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -815,6 +815,9 @@ static void printMI(raw_ostream &OS, MFPrintState &State,
if (MI.getFlag(MachineInstr::SameSign))
OS << "samesign ";
+ // NOTE: Please add new MIFlags also to the MI_FLAGS_STR in
+ // llvm/utils/update_mir_test_checks.py.
+
OS << TII->getName(MI.getOpcode());
LS = ListSeparator();
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-gep-flags.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-gep-flags.ll
index 34ac4f6361d96..8a6f2664f1e88 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-gep-flags.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-gep-flags.ll
@@ -17,8 +17,8 @@ define i32 @gep_nusw_nuw(ptr %ptr, i32 %idx) {
; CHECK-NEXT: [[MUL1:%[0-9]+]]:_(s64) = G_MUL [[SEXT]], [[C]]
; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[COPY]], [[MUL1]](s64)
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
- ; CHECK-NEXT: %11:_(p0) = nuw nusw G_PTR_ADD [[PTR_ADD1]], [[C1]](s64)
- ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s32) = G_LOAD %11(p0) :: (load (s32) from %ir.gep2)
+ ; CHECK-NEXT: [[PTR_ADD2:%[0-9]+]]:_(p0) = nuw nusw G_PTR_ADD [[PTR_ADD1]], [[C1]](s64)
+ ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[PTR_ADD2]](p0) :: (load (s32) from %ir.gep2)
; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[LOAD]], [[LOAD1]]
; CHECK-NEXT: $w0 = COPY [[ADD]](s32)
; CHECK-NEXT: RET_ReallyLR implicit $w0
@@ -77,8 +77,8 @@ define i32 @gep_nusw(ptr %ptr, i32 %idx) {
; CHECK-NEXT: [[MUL1:%[0-9]+]]:_(s64) = G_MUL [[SEXT]], [[C]]
; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[COPY]], [[MUL1]](s64)
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 4
- ; CHECK-NEXT: %11:_(p0) = nusw G_PTR_ADD [[PTR_ADD1]], [[C1]](s64)
- ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s32) = G_LOAD %11(p0) :: (load (s32) from %ir.gep2)
+ ; CHECK-NEXT: [[PTR_ADD2:%[0-9]+]]:_(p0) = nusw G_PTR_ADD [[PTR_ADD1]], [[C1]](s64)
+ ; CHECK-NEXT: [[LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[PTR_ADD2]](p0) :: (load (s32) from %ir.gep2)
; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[LOAD]], [[LOAD1]]
; CHECK-NEXT: $w0 = COPY [[ADD]](s32)
; CHECK-NEXT: RET_ReallyLR implicit $w0
diff --git a/llvm/utils/update_mir_test_checks.py b/llvm/utils/update_mir_test_checks.py
index 8db46ad4ebc95..3078f7a5e3ec5 100755
--- a/llvm/utils/update_mir_test_checks.py
+++ b/llvm/utils/update_mir_test_checks.py
@@ -35,7 +35,8 @@
VREG_RE = re.compile(r"(%[0-9]+)(?:\.[a-z0-9_]+)?(?::[a-z0-9_]+)?(?:\([<>a-z0-9 ]+\))?")
MI_FLAGS_STR = (
r"(frame-setup |frame-destroy |nnan |ninf |nsz |arcp |contract |afn "
- r"|reassoc |nuw |nsw |exact |nofpexcept |nomerge |disjoint )*"
+ r"|reassoc |nuw |nsw |exact |nofpexcept |nomerge |unpredictable "
+ r"|noconvergent |nneg |disjoint |nusw |samesign )*"
)
VREG_DEF_FLAGS_STR = r"(?:dead |undef )*"
VREG_DEF_RE = re.compile(
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As an alternative could we assume that any lower case string is an MIFlag and that instruction names are in upper case? Or would that be too fragile?
I like this idea, but it seems opcodes currently can start either with an upper-case letter or with a lower-case Opcodes that are not entirely upper-case do exist, we have at least some like |
Arm uses 't' for Thumb instructions and 't2' for Thumb2 instructions I'm afraid. I'm not sure why that was used but it helps distinguish them from the normal "Arm" mode instructions, like |
…tter or a lower-case 't'
Thanks, that's good to know! I've updated the PR to add a comment explaining that in the script. So, we could probably use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we go with this version and update it in the future if needed?
Merge activity
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/20233 Here is the relevant piece of the build log for the reference
|
I'm pretty sure this PR didn't change anything about the availability of |
If the update_mir_test_checks.py script is aware of MIFlags, it can produce meaningful identifiers in generated FileCheck lines. A few MIFlags that were introduced more recently have been missing from the script. Ideally, the MIFlags would be specified in a single place and automatically made known to the script to avoid this divergence, but for now adding a comment pointing to the script at the place where the MIFlags are printed seems like a reasonable trade-off. This PR only regenerates check lines for a single test as an example of the effect; other affected tests are not regenerated for now to avoid unnecessary test churn.
If the update_mir_test_checks.py script is aware of MIFlags, it can produce
meaningful identifiers in generated FileCheck lines. A few MIFlags that were
introduced more recently have been missing from the script.
Ideally, the MIFlags would be specified in a single place and automatically
made known to the script to avoid this divergence, but for now adding a comment
pointing to the script at the place where the MIFlags are printed seems like a
reasonable trade-off.
This PR only regenerates check lines for a single test as an example of the
effect; other affected tests are not regenerated for now to avoid unnecessary
test churn.