Page MenuHomePhabricator

GlobalISel: Add combines for G_TRUNC
ClosedPublic

Authored by volkan on Sep 2 2020, 12:11 PM.

Details

Diff Detail

Event Timeline

volkan created this revision.Sep 2 2020, 12:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2020, 12:11 PM
volkan requested review of this revision.Sep 2 2020, 12:11 PM
arsenm added inline comments.Sep 2 2020, 12:22 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1845

No else after return

1867

hasOneNonDBGUse?

1869

For AMDGPU this would need a regbank legality check for post-regbankselect combines but I'm not sure what to do about that

1891

buildShl

llvm/test/CodeGen/AMDGPU/GlobalISel/shl.ll
85–87

This is strange looking. It's technically neutral but the shift amount no longer appears constant?

volkan added inline comments.Sep 2 2020, 3:30 PM
llvm/test/CodeGen/AMDGPU/GlobalISel/shl.ll
85–87

Here is the MIR after AMDGPUPostLegalizerCombiner:

bb.1 (%ir-block.0):
  liveins: $sgpr0
  %2:_(s32) = COPY $sgpr0
  %1:_(s16) = G_TRUNC %2:_(s32)
  %5:_(s32) = G_CONSTANT i32 7
  %16:_(s16) = G_TRUNC %5:_(s32)
  %15:_(s16) = nuw nsw G_SHL %1:_, %16:_(s16)
  %8:_(s32) = G_ANYEXT %15:_(s16)
  %9:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), %8:_(s32)
  $sgpr0 = COPY %9:_(s32)
  SI_RETURN_TO_EPILOG implicit $sgpr0

Looks like it's not constant because there is no combine to transform trunc(cst) to cst. We should add that as a part of the constant folding combines.

volkan updated this revision to Diff 289590.Sep 2 2020, 3:33 PM

Updated the patch based on the feedback.

volkan marked 3 inline comments as done.Sep 2 2020, 3:34 PM

@arsenm

For AMDGPU this would need a regbank legality check for post-regbankselect combines but I'm not sure what to do about that

Maybe it would make sense for now to add something like

SelectionStage getSelectionStage(MachineFunction &MF);
...
if (getSelectionStage(MF) < SelectionStage::Legalized)
   return false;
...
if (getSelectionStage(MF) != SelectionStage::RegBankSelected)
   return false;
...

So we can at least bail out when something could be unsafe for some target?

Ultimately, it would be nicest to declare which stages each combine is valid/invalid at in the tablegen somehow, but I guess something like this could be useful temporarily? :/

arsenm accepted this revision.Sep 14 2020, 4:01 PM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1864

Sink this down into the query statement?

This revision is now accepted and ready to land.Sep 14 2020, 4:01 PM