[PATCH] D61289: [globalisel] Add G_SEXT_INREG

Mon Apr 29 17:31:30 PDT 2019

dsanders created this revision.
dsanders added reviewers: bogner, aditya_nandakumar, volkan, aemerson, paquette, arsenm.
Herald added subscribers: Petar.Avramovic, atanasyan, jrtc27, hiraditya, javed.absar, kristof.beyls, rovka, nhaehnle, wdng, jvesely, sdardis.
Herald added a project: LLVM.

Targets often have instructions that can sign-extend certain cases faster
than the equivalent shift-left/arithmetic-shift-right. Such cases can be
identified by matching a shift-left/shift-right pair but there are some
issues with this in the context of combines. For example, suppose you can
sign-extend 8-bit up to 32-bit with a target extend instruction.

  %1:_(s32) = G_SHL %0:_(s32), i32 24 # (I've inlined the G_CONSTANT for brevity)
  %2:_(s32) = G_ASHR %1:_(s32), i32 24
  %3:_(s32) = G_ASHR %2:_(s32), i32 1

would reasonably combine to:

  %1:_(s32) = G_SHL %0:_(s32), i32 24
  %2:_(s32) = G_ASHR %1:_(s32), i32 25

which no longer matches the special case. If your shifts and extend are
equal cost, this would break even as a pair of shifts but if your shift is
more expensive than the extend then it's cheaper as:

  %2:_(s32) = G_SEXT_INREG %0:_(s32), i32 8
  %3:_(s32) = G_ASHR %2:_(s32), i32 1

It's possible to match the shift-pair in ISel and emit an extend and ashr.
However, this is far from the only way to break this shift pair and make
it hard to match the extends. Another example is that with the right
known-zeros, this:

  %1:_(s32) = G_SHL %0:_(s32), i32 24
  %2:_(s32) = G_ASHR %1:_(s32), i32 24
  %3:_(s32) = G_MUL %2:_(s32), i32 2

can become:

  %1:_(s32) = G_SHL %0:_(s32), i32 24
  %2:_(s32) = G_ASHR %1:_(s32), i32 23

All upstream targets have been configured to lower it to the current
G_SHL,G_ASHR pair but will likely want to make it legal in some cases to
handle their faster cases.

To follow-up: Provide a way to legalize based on the constant. At the
moment, I'm thinking that the best way to achieve this is to provide the
MI in LegalityQuery but that opens the door to breaking core principles
of the legalizer (legality is not context sensitive). That said, it's
worth noting that looking at other instructions and acting on that
information doesn't violate this principle in itself. It's only a
violation if, at the end of legalization, a pass that checks legality
without being able to see the context would say an instruction might not be
legal. That's a fairly subtle distinction so to give a concrete example,
saying %2 in:

  %1 = G_CONSTANT 16
  %2 = G_SEXT_INREG %0, %1

is legal is in violation of that principle if the legality of %2 depends
on %1 being constant and/or being 16. However, legalizing to either:

  %2 = G_SEXT_INREG %0, 16

or:

  %1 = G_CONSTANT 16
  %2:_(s32) = G_SHL %0, %1
  %3:_(s32) = G_ASHR %2, %1

depending on whether %1 is constant and 16 does not violate that principle
since both outputs are genuinely legal.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61289

Files:
  llvm/include/llvm/CodeGen/GlobalISel/ConstantFoldingMIRBuilder.h
  llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
  llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
  llvm/include/llvm/CodeGen/GlobalISel/Utils.h
  llvm/include/llvm/Support/TargetOpcodes.def
  llvm/include/llvm/Target/GenericOpcodes.td
  llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/lib/CodeGen/GlobalISel/Utils.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/lib/Target/ARM/ARMLegalizerInfo.cpp
  llvm/lib/Target/Mips/MipsLegalizerInfo.cpp
  llvm/lib/Target/X86/X86LegalizerInfo.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extends.ll
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-div.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-ext.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-gep.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-itofp.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-rem.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-sext.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-shift.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-undef.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
  llvm/test/CodeGen/AArch64/GlobalISel/verify-g_sext_inreg.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/artifact-combiner-sext.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/combine-ext-legalizer.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-ashr.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-extract-vector-elt.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sext.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sextload-flat.mir
  llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-divmod.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/add.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/constants.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/mul.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/rem_and_div.mir
  llvm/test/CodeGen/Mips/GlobalISel/legalizer/sub.mir
  llvm/test/CodeGen/X86/GlobalISel/legalize-ext-x86-64.mir
  llvm/test/CodeGen/X86/GlobalISel/x86_64-legalize-sitofp.mir
  llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
  llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61289.197230.patch
Type: text/x-patch
Size: 131698 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190430/ca8df1b9/attachment.bin>