Page MenuHomePhabricator
Differential D125836

[DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m)
ClosedPublic
Actions

Authored by RKSimon on May 17 2022, 2:18 PM.

Details

Reviewers
dmgreen
foad
craig.topper
efriedma
spatel
Commits
rGe4a124dda593: [DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m)
Summary

Similar to the existing (shl (srl x, c1), c2) fold

Part of the work to fix the regressions in D77804

Diff Detail

Unit TestsFailed
View All

TimeTest
60,050 msx64 debian > Clang.CodeGenCXX::dllimport-members.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -no-opaque-pointers -no-enable-noundef-analysis -disable-llvm-passes -triple i686-windows-msvc -fms-compatibility -emit-llvm -std=c++1y -O0 -o - /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/dllimport-members.cpp -DMSABI | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=MSC --check-prefix=M32 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/dllimport-members.cpp
60,790 msx64 debian > Clang.Driver::arm-cortex-cpus-2.c
Script: -- : 'RUN: at line 8'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -target armv8a-linux-eabi -mcpu=cortex-a53+fp16 -### -c /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-2.c 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix CHECK-CORTEX-A53-FP16 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-2.c
60,920 msx64 debian > Clang.Driver::fsanitize.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-trap=undefined /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/fsanitize.c -### 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/fsanitize.c --check-prefix=CHECK-UNDEFINED-TRAP
60,150 msx64 debian > Clang.OpenMP::target_teams_distribute_parallel_for_simd_codegen_registration.cpp
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -no-opaque-pointers -verify -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --allow-unused-prefixes /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
61,030 msx64 debian > Clang.OpenMP::target_update_codegen.cpp
Script: -- : 'RUN: at line 6'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -no-opaque-pointers -DCK1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/target_update_codegen.cpp -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --allow-unused-prefixes /var/lib/buildkite-agent/builds/llvm-project/clang/test/OpenMP/target_update_codegen.cpp --check-prefix CK1 --check-prefix CK1-64

Event Timeline

RKSimon created this revision.May 17 2022, 2:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 2:18 PM
Herald added subscribers: kosarev, StephenFan, ecnelises and 4 others. · View Herald Transcript
RKSimon requested review of this revision.May 17 2022, 2:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 2:18 PM
Harbormaster completed remote builds in B164982: Diff 430184.May 17 2022, 3:52 PM
RKSimon updated this revision to Diff 430476.May 18 2022, 12:28 PM

rebase

Harbormaster completed remote builds in B165189: Diff 430476.May 18 2022, 1:05 PM
RKSimon mentioned this in D126117: [DAG] SimplifyDemandedBits - ISD::OR - attempt to simplify Op1 again once we have KnownBits from Op0.May 21 2022, 5:50 AM
RKSimon mentioned this in D127598: [DAG] SimplifyDemandedBits OR(Op0,Op1) - attempt to simplify Op1 again once we have the known bits of Op0.Jun 12 2022, 7:05 AM
RKSimon mentioned this in D127603: [AArch64] isSeveralBitsExtractOpFromShr - match UBFM patterns with value tracking (RFC).Jun 12 2022, 9:21 AM
RKSimon mentioned this in rG1cf9b24da3b8: [DAG] Enable ISD::FSHL/R SimplifyMultipleUseDemandedBits handling inside….Jun 12 2022, 11:26 AM
RKSimon updated this revision to Diff 436239.Jun 12 2022, 11:26 AM

rebase

Herald added a subscriber: jsji. · View Herald TranscriptJun 12 2022, 11:26 AM
Harbormaster completed remote builds in B169325: Diff 436239.Jun 12 2022, 12:33 PM
foad added a comment.Jun 13 2022, 2:08 AM

Surely the (srl (shl x, c1), c2) form is better for targets which aren't good at doing an AND-with-large-immediate-value. Is this supposed to be a canonicalization, which targets will undo later as appropriate?

RKSimon added a comment.Jun 13 2022, 2:09 AM
In D125836#3577140, @foad wrote:

Surely the (srl (shl x, c1), c2) form is better for targets which aren't good at doing an AND-with-large-immediate-value. Is this supposed to be a canonicalization, which targets will undo later as appropriate?

We have the TLI shouldFoldConstantShiftPairToMask callback to allow targets to control this.

craig.topper added inline comments.Jun 14 2022, 10:37 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9437

Why is the last SDLoc from N0?

RKSimon added inline comments.Jun 14 2022, 10:48 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9437

It was copy+pasted from the (shl (srl x, c1), c2) variant

craig.topper added inline comments.Jun 14 2022, 10:58 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9437

Is that correct? Can you swap the order of debug locs? The N0 debug loc is now after the N location in the DAG

RKSimon added inline comments.Jun 14 2022, 11:02 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9437

I'll take a look

RKSimon mentioned this in D127772: [DAG] Fix SDLoc mismatch in (shl (srl x, c1), c2) -> and(shift(x,c3)) fold.Jun 14 2022, 11:13 AM
RKSimon added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9437

D127772

RKSimon mentioned this in rGf096d5926ddc: [DAG] Fix SDLoc mismatch in (shl (srl x, c1), c2) -> and(shift(x,c3)) fold.Jun 15 2022, 3:08 AM
RKSimon updated this revision to Diff 437103.Jun 15 2022, 3:24 AM

Fix SDLoc mismatch

Harbormaster completed remote builds in B169946: Diff 437103.Jun 15 2022, 4:11 AM
RKSimon updated this revision to Diff 437916.Jun 17 2022, 8:18 AM
RKSimon retitled this revision from [DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m) (WIP) to [DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m).
RKSimon edited the summary of this revision. (Show Details)

Extend AArch64TargetLowering::shouldFoldConstantShiftPairToMask so that we don't lose the srl(shl(x,c1),c2) pair if it would likely break a UBFX pattern.

Harbormaster completed remote builds in B170522: Diff 437916.Jun 17 2022, 9:47 AM
dmgreen added a comment.Jun 19 2022, 2:50 AM

The AArch64 change sounds OK to me.

RKSimon added a comment.Jun 19 2022, 4:41 AM

Thank @dmgreen - does anyone have more comments?

RKSimon added a reviewer: spatel.Jun 19 2022, 10:04 AM
spatel accepted this revision.Jun 19 2022, 12:21 PM

LGTM - noted a close call in x86 tests, but that can be addressed if we find real regressions.

llvm/test/CodeGen/X86/shift-mask.ll
597

This and the next diff might be worse depending on uarch, so that suggests a possible refinement for the x86 TLI hook for 64-bit integer.

This revision is now accepted and ready to land.Jun 19 2022, 12:21 PM
This revision was landed with ongoing or failed builds.Jun 20 2022, 12:37 AM
Closed by commit rGe4a124dda593: [DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m) (authored by RKSimon). · Explain Why
This revision was automatically updated to reflect the committed changes.
RKSimon added a commit: rGe4a124dda593: [DAG] Fold (srl (shl x, c1), c2) -> and(shl/srl(x, c3), m).
RKSimon mentioned this in D77804: [DAG] Enable ISD::SRL SimplifyMultipleUseDemandedBits handling inside SimplifyDemandedBits.Jun 20 2022, 1:36 AM

Revision Contents
Changeset List

PathSize
llvm/
lib/
CodeGen/
SelectionDAG/
46 lines
test/
CodeGen/
AArch64/
10 lines
5 lines
9 lines
AMDGPU/
14 lines
122 lines
20 lines
8 lines
ARM/
17 lines
X86/
5 lines
5 lines
20 lines
16 lines
148 lines

Diff 430476

View Options

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Loading...
View Options

llvm/test/CodeGen/AArch64/arm64-bitfield-extract.ll

Loading...
View Options

llvm/test/CodeGen/AArch64/arm64-rev.ll

Loading...
View Options

llvm/test/CodeGen/AArch64/ushl_sat.ll

Loading...
View Options

llvm/test/CodeGen/AMDGPU/idot4s.ll

Loading...
View Options

llvm/test/CodeGen/AMDGPU/idot8s.ll

Loading...
View Options

llvm/test/CodeGen/AMDGPU/idot8u.ll

Loading...
View Options

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ubfe.ll

Loading...
View Options

llvm/test/CodeGen/ARM/umulo-32.ll

Loading...
View Options

llvm/test/CodeGen/X86/pr32588.ll

Loading...
View Options

llvm/test/CodeGen/X86/pull-binop-through-shift.ll

Loading...
View Options

llvm/test/CodeGen/X86/rotate-extract-vector.ll

Loading...
View Options

llvm/test/CodeGen/X86/rotate-extract.ll

Loading...
View Options

llvm/test/CodeGen/X86/shift-mask.ll

Loading...