Simplify each operand of AND and OR using demanded bits based on the
known bits of the other operand.
Details
Details
- Reviewers
RKSimon craig.topper
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1220 | If we're worried about the cost of the extra SimplifyDemandedBits, we might be able to just create Op0DemandedBits/Op1DemandedBits from the knowbits and only use these on the SimplifyMultipleUseDemandedBits cases below? The calls currently only use the original DemandedBits. | |
Comment Actions
Not seeing any impact from this change (https://llvm-compile-time-tracker.com/compare.php?from=a5168bdb4a25485ac62e18bdc538b4842bc9fbd9&to=c23aa3c1c163fc5343b1df5254b8c0782d4324cb&stat=instructions).
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1220 | I did try using Op0DemandedBits/Op1DemandedBits for the calls to SimplifyMultipleUseDemandedBits, in addition to the current patch. It only affected one test case and the diff isn't very compelling: diff --git a/llvm/test/CodeGen/X86/shift-parts.ll b/llvm/test/CodeGen/X86/shift-parts.ll index da00f377020d..ef82065ad12a 100644 --- a/llvm/test/CodeGen/X86/shift-parts.ll +++ b/llvm/test/CodeGen/X86/shift-parts.ll @@ -12,11 +12,12 @@ define i32 @int87(i32 %uint64p_8, i1 %cond) nounwind { ; CHECK-NEXT: movq g_144+{{.*}}(%rip), %rax ; CHECK-NEXT: movq g_144+{{.*}}(%rip), %rcx ; CHECK-NEXT: movzbl %sil, %edx +; CHECK-NEXT: andl $1, %edx ; CHECK-NEXT: shll $6, %edx ; CHECK-NEXT: .p2align 4, 0x90 ; CHECK-NEXT: .LBB0_1: # %for.cond ; CHECK-NEXT: # =>This Inner Loop Header: Depth=1 -; CHECK-NEXT: testb $64, %dl +; CHECK-NEXT: testb %dl, %dl ; CHECK-NEXT: movq %rcx, %rsi ; CHECK-NEXT: cmovneq %rax, %rsi ; CHECK-NEXT: orl $0, %esi | |
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1271 | Can we trust Known2.One's value is correct for the bits that weren't checked due to Known.One being used to filter the DemandedBits for the previous call? | |
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1271 | No we can't according to the comment on SimplifyDemandedBits: /// expression (used to simplify the caller). The KnownZero/One bits may only /// be accurate for those bits in the Demanded masks. This was a pre-existing problem with the call on line 1264. | |
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1271 | It seems to me that there are various pre-existing problems in this area. This should be a no-op but causes test case diffs: https://github.com/jayfoad/llvm-project/commit/dfdc519d44ac4f5ad00d9d5322d641e2fcc5881a This fixes the pre-existing problem I pointed out on line 1264, but causes various code quality regressions: https://github.com/jayfoad/llvm-project/commit/90a5c28ba29edf54fb22ea36bf1ba2625ce557a4 I'm not sure how to proceed now. Should we try to get to a state where known bits information is trustworthy for undemanded bits? | |
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1271 | If you're willing to try to do the audit then by all means - we should at least be able to get to a position where all the KnownBits One/Zero bits will not be set unless they are definitely known - but some cases (e.g. bitcasts through vectors, partial demanded elts etc.) we might need to play safe. | |
| llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp | ||
|---|---|---|
| 1225 | Done. The only effect on the tests is to add an instruction outside the loop in X86/shift-parts.ll which does not seem like a significant regression to me. | |
Comment Actions
The status is: I'm still nervous about this patch because of the issue @craig.topper pointed out with SimplifyDemandedBits not necessarily returning accurate "known" information for bits that are not demanded.
Pull out