[GlobalISel] Adding support for handling G_ASSERT_{SEXT,ZEXT,ALIGN} i… #74196
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
…n artifact combiner These instructions are hint for optimizations and can be treated as copies and are handled as such with this change. Without it is possible to run into an assertion, since tryCombineUnmergeValues rightfully use getDefIgnoringCopies to get the source MI, which already handle these hint instructions and treat them as copies. The problem is that markDefDead only considers COPYs, which will lead to crash with assertion for cases like testUnmergeHintOfTrunc.
Member
| @llvm/pr-subscribers-llvm-globalisel Author: Dávid Ferenc Szabó (dfszabo) Changes…n artifact combiner These instructions are hint for optimizations and can be treated as copies and are handled as such with this change. Without it is possible to run into an assertion, since tryCombineUnmergeValues rightfully use getDefIgnoringCopies to get the source MI, which already handle these hint instructions and treat them as copies. The problem is that markDefDead only considers COPYs, which will lead to crash with assertion for cases like testUnmergeHintOfTrunc. Full diff: https://github.com/llvm/llvm-project/pull/74196.diff 3 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h index 851353042cc26..da330b517c280 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h @@ -1366,6 +1366,9 @@ class LegalizationArtifactCombiner { // Adding Use to ArtifactList. WrapperObserver.changedInstr(Use); break; + case TargetOpcode::G_ASSERT_SEXT: + case TargetOpcode::G_ASSERT_ZEXT: + case TargetOpcode::G_ASSERT_ALIGN: case TargetOpcode::COPY: { Register Copy = Use.getOperand(0).getReg(); if (Copy.isVirtual()) @@ -1392,6 +1395,9 @@ class LegalizationArtifactCombiner { case TargetOpcode::G_ANYEXT: case TargetOpcode::G_SEXT: case TargetOpcode::G_EXTRACT: + case TargetOpcode::G_ASSERT_SEXT: + case TargetOpcode::G_ASSERT_ZEXT: + case TargetOpcode::G_ASSERT_ALIGN: return MI.getOperand(1).getReg(); case TargetOpcode::G_UNMERGE_VALUES: return MI.getOperand(MI.getNumOperands() - 1).getReg(); @@ -1425,7 +1431,8 @@ class LegalizationArtifactCombiner { if (MRI.hasOneUse(PrevRegSrc)) { if (TmpDef != &DefMI) { assert((TmpDef->getOpcode() == TargetOpcode::COPY || - isArtifactCast(TmpDef->getOpcode())) && + isArtifactCast(TmpDef->getOpcode()) || + isPreISelGenericOptimizationHint(TmpDef->getOpcode())) && "Expecting copy or artifact cast here"); DeadInsts.push_back(TmpDef); @@ -1509,16 +1516,8 @@ class LegalizationArtifactCombiner { /// Looks through copy instructions and returns the actual /// source register. Register lookThroughCopyInstrs(Register Reg) { - using namespace llvm::MIPatternMatch; - - Register TmpReg; - while (mi_match(Reg, MRI, m_Copy(m_Reg(TmpReg)))) { - if (MRI.getType(TmpReg).isValid()) - Reg = TmpReg; - else - break; - } - return Reg; + Register TmpReg = getSrcRegIgnoringCopies(Reg, MRI); + return TmpReg.isValid() ? TmpReg : Reg; } }; diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll index a7d7a1e81617e..5867326c18aa6 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll +++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll @@ -46,14 +46,11 @@ define i32 @test(i32 %a, i1 %c) { ; PRESELECTION-NEXT: {{ $}} ; PRESELECTION-NEXT: [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0 ; PRESELECTION-NEXT: [[COPY1:%[0-9]+]]:gpr(s32) = COPY $w1 - ; PRESELECTION-NEXT: [[TRUNC:%[0-9]+]]:gpr(s8) = G_TRUNC [[COPY1]](s32) - ; PRESELECTION-NEXT: [[ASSERT_ZEXT:%[0-9]+]]:gpr(s8) = G_ASSERT_ZEXT [[TRUNC]], 1 ; PRESELECTION-NEXT: [[C:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 0 ; PRESELECTION-NEXT: [[C1:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 100000 ; PRESELECTION-NEXT: [[CONSTANT_FOLD_BARRIER:%[0-9]+]]:gpr(s32) = G_CONSTANT_FOLD_BARRIER [[C1]] - ; PRESELECTION-NEXT: [[ANYEXT:%[0-9]+]]:gpr(s32) = G_ANYEXT [[ASSERT_ZEXT]](s8) ; PRESELECTION-NEXT: [[C2:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 1 - ; PRESELECTION-NEXT: [[AND:%[0-9]+]]:gpr(s32) = G_AND [[ANYEXT]], [[C2]] + ; PRESELECTION-NEXT: [[AND:%[0-9]+]]:gpr(s32) = G_AND [[COPY1]], [[C2]] ; PRESELECTION-NEXT: G_BRCOND [[AND]](s32), %bb.3 ; PRESELECTION-NEXT: G_BR %bb.2 ; PRESELECTION-NEXT: {{ $}} diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir index 62865bcfdf081..ce02aa380efe1 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir @@ -136,6 +136,47 @@ body: | $x0 = COPY %3(s64) RET_ReallyLR implicit $x0 +... +--- +name: testExtOfHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testExtOfHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: $x0 = COPY [[COPY]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s8) = G_TRUNC %0(s64) + %2:_(s8) = G_ASSERT_ZEXT %1(s8), 1 + %3:_(s64) = G_ANYEXT %2(s8) + $x0 = COPY %3(s64) + RET_ReallyLR implicit $x0 + +... +--- +name: testExtOfCopyAndHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testExtOfCopyAndHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: $x0 = COPY [[COPY]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s8) = G_TRUNC %0(s64) + %2:_(s8) = COPY %1(s8) + %3:_(s8) = G_ASSERT_ZEXT %2(s8), 1 + %4:_(s64) = G_ANYEXT %3(s8) + $x0 = COPY %4(s64) + RET_ReallyLR implicit $x0 + ... --- name: testExtOf2CopyOfTrunc @@ -158,3 +199,26 @@ body: | RET_ReallyLR implicit $x0 ... +--- +name: testUnmergeHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testUnmergeHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: [[UV:%[0-9]+]]:_(s8), [[UV1:%[0-9]+]]:_(s8), [[UV2:%[0-9]+]]:_(s8), [[UV3:%[0-9]+]]:_(s8), [[UV4:%[0-9]+]]:_(s8), [[UV5:%[0-9]+]]:_(s8), [[UV6:%[0-9]+]]:_(s8), [[UV7:%[0-9]+]]:_(s8) = G_UNMERGE_VALUES [[COPY]](s64) + ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[UV1]](s8) + ; CHECK-NEXT: $x0 = COPY [[ANYEXT]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s16) = G_TRUNC %0(s64) + %2:_(s16) = G_ASSERT_ZEXT %1(s16), 1 + %3:_(s8), %4:_(s8) = G_UNMERGE_VALUES %2(s16) + %5:_(s64) = G_ANYEXT %4(s8) + $x0 = COPY %5(s64) + RET_ReallyLR implicit $x0 + +... |
Member
| @llvm/pr-subscribers-backend-aarch64 Author: Dávid Ferenc Szabó (dfszabo) Changes…n artifact combiner These instructions are hint for optimizations and can be treated as copies and are handled as such with this change. Without it is possible to run into an assertion, since tryCombineUnmergeValues rightfully use getDefIgnoringCopies to get the source MI, which already handle these hint instructions and treat them as copies. The problem is that markDefDead only considers COPYs, which will lead to crash with assertion for cases like testUnmergeHintOfTrunc. Full diff: https://github.com/llvm/llvm-project/pull/74196.diff 3 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h index 851353042cc26..da330b517c280 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h @@ -1366,6 +1366,9 @@ class LegalizationArtifactCombiner { // Adding Use to ArtifactList. WrapperObserver.changedInstr(Use); break; + case TargetOpcode::G_ASSERT_SEXT: + case TargetOpcode::G_ASSERT_ZEXT: + case TargetOpcode::G_ASSERT_ALIGN: case TargetOpcode::COPY: { Register Copy = Use.getOperand(0).getReg(); if (Copy.isVirtual()) @@ -1392,6 +1395,9 @@ class LegalizationArtifactCombiner { case TargetOpcode::G_ANYEXT: case TargetOpcode::G_SEXT: case TargetOpcode::G_EXTRACT: + case TargetOpcode::G_ASSERT_SEXT: + case TargetOpcode::G_ASSERT_ZEXT: + case TargetOpcode::G_ASSERT_ALIGN: return MI.getOperand(1).getReg(); case TargetOpcode::G_UNMERGE_VALUES: return MI.getOperand(MI.getNumOperands() - 1).getReg(); @@ -1425,7 +1431,8 @@ class LegalizationArtifactCombiner { if (MRI.hasOneUse(PrevRegSrc)) { if (TmpDef != &DefMI) { assert((TmpDef->getOpcode() == TargetOpcode::COPY || - isArtifactCast(TmpDef->getOpcode())) && + isArtifactCast(TmpDef->getOpcode()) || + isPreISelGenericOptimizationHint(TmpDef->getOpcode())) && "Expecting copy or artifact cast here"); DeadInsts.push_back(TmpDef); @@ -1509,16 +1516,8 @@ class LegalizationArtifactCombiner { /// Looks through copy instructions and returns the actual /// source register. Register lookThroughCopyInstrs(Register Reg) { - using namespace llvm::MIPatternMatch; - - Register TmpReg; - while (mi_match(Reg, MRI, m_Copy(m_Reg(TmpReg)))) { - if (MRI.getType(TmpReg).isValid()) - Reg = TmpReg; - else - break; - } - return Reg; + Register TmpReg = getSrcRegIgnoringCopies(Reg, MRI); + return TmpReg.isValid() ? TmpReg : Reg; } }; diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll index a7d7a1e81617e..5867326c18aa6 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll +++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll @@ -46,14 +46,11 @@ define i32 @test(i32 %a, i1 %c) { ; PRESELECTION-NEXT: {{ $}} ; PRESELECTION-NEXT: [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0 ; PRESELECTION-NEXT: [[COPY1:%[0-9]+]]:gpr(s32) = COPY $w1 - ; PRESELECTION-NEXT: [[TRUNC:%[0-9]+]]:gpr(s8) = G_TRUNC [[COPY1]](s32) - ; PRESELECTION-NEXT: [[ASSERT_ZEXT:%[0-9]+]]:gpr(s8) = G_ASSERT_ZEXT [[TRUNC]], 1 ; PRESELECTION-NEXT: [[C:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 0 ; PRESELECTION-NEXT: [[C1:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 100000 ; PRESELECTION-NEXT: [[CONSTANT_FOLD_BARRIER:%[0-9]+]]:gpr(s32) = G_CONSTANT_FOLD_BARRIER [[C1]] - ; PRESELECTION-NEXT: [[ANYEXT:%[0-9]+]]:gpr(s32) = G_ANYEXT [[ASSERT_ZEXT]](s8) ; PRESELECTION-NEXT: [[C2:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 1 - ; PRESELECTION-NEXT: [[AND:%[0-9]+]]:gpr(s32) = G_AND [[ANYEXT]], [[C2]] + ; PRESELECTION-NEXT: [[AND:%[0-9]+]]:gpr(s32) = G_AND [[COPY1]], [[C2]] ; PRESELECTION-NEXT: G_BRCOND [[AND]](s32), %bb.3 ; PRESELECTION-NEXT: G_BR %bb.2 ; PRESELECTION-NEXT: {{ $}} diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir index 62865bcfdf081..ce02aa380efe1 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-simple.mir @@ -136,6 +136,47 @@ body: | $x0 = COPY %3(s64) RET_ReallyLR implicit $x0 +... +--- +name: testExtOfHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testExtOfHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: $x0 = COPY [[COPY]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s8) = G_TRUNC %0(s64) + %2:_(s8) = G_ASSERT_ZEXT %1(s8), 1 + %3:_(s64) = G_ANYEXT %2(s8) + $x0 = COPY %3(s64) + RET_ReallyLR implicit $x0 + +... +--- +name: testExtOfCopyAndHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testExtOfCopyAndHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: $x0 = COPY [[COPY]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s8) = G_TRUNC %0(s64) + %2:_(s8) = COPY %1(s8) + %3:_(s8) = G_ASSERT_ZEXT %2(s8), 1 + %4:_(s64) = G_ANYEXT %3(s8) + $x0 = COPY %4(s64) + RET_ReallyLR implicit $x0 + ... --- name: testExtOf2CopyOfTrunc @@ -158,3 +199,26 @@ body: | RET_ReallyLR implicit $x0 ... +--- +name: testUnmergeHintOfTrunc +body: | + bb.0: + liveins: $x0 + + ; CHECK-LABEL: name: testUnmergeHintOfTrunc + ; CHECK: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x0 + ; CHECK-NEXT: [[UV:%[0-9]+]]:_(s8), [[UV1:%[0-9]+]]:_(s8), [[UV2:%[0-9]+]]:_(s8), [[UV3:%[0-9]+]]:_(s8), [[UV4:%[0-9]+]]:_(s8), [[UV5:%[0-9]+]]:_(s8), [[UV6:%[0-9]+]]:_(s8), [[UV7:%[0-9]+]]:_(s8) = G_UNMERGE_VALUES [[COPY]](s64) + ; CHECK-NEXT: [[ANYEXT:%[0-9]+]]:_(s64) = G_ANYEXT [[UV1]](s8) + ; CHECK-NEXT: $x0 = COPY [[ANYEXT]](s64) + ; CHECK-NEXT: RET_ReallyLR implicit $x0 + %0:_(s64) = COPY $x0 + %1:_(s16) = G_TRUNC %0(s64) + %2:_(s16) = G_ASSERT_ZEXT %1(s16), 1 + %3:_(s8), %4:_(s8) = G_UNMERGE_VALUES %2(s16) + %5:_(s64) = G_ANYEXT %4(s8) + $x0 = COPY %5(s64) + RET_ReallyLR implicit $x0 + +... |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
…n artifact combiner
These instructions are hint for optimizations and can be treated as copies and are handled as such with this change. Without it is possible to run into an assertion, since tryCombineUnmergeValues rightfully use getDefIgnoringCopies to get the source MI, which already handle these hint instructions and treat them as copies. The problem is that markDefDead only considers COPYs, which will lead to crash with assertion for cases like testUnmergeHintOfTrunc.