[AMDGPU] Add a new Clamp Pattern to the GlobalISel Path.

[tsymalla]

Summary

This patch should find a pattern in GlobalISel / AMDGPUPostLegalizerCombiner which can appear when trying to clamp 64-bit values to short values without truncation, for example by using the 64-bit / 16-bit integer extensions in shaders. This pattern can be reduced to two VALU instructions:

v_cvt_pk_i16_i32_e64 v0, v0, v1 ; where v[0:1] is a 64-bit number
v_med3_i32 v0, LowBoundary, v0, HighBoundary ; where LowBoundary >= SHORT_MII, HighBoundary <= SHORT_MAX

I am open for suggestions and reviews as this is my first change on the LLVM compiler.

[Flakebi]

Looks good to me, I left some nits inline.
Someone who is more familiar with GlobalISel should review this.

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

1–2

Unintended change?

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

3 ↗

(On Diff #313345)

GFX9 is also a GCN architecture, so calling this GFX10 might be a clearer choice.

[Petar.Avramovic]

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

11 ↗

(On Diff #313345)

This looks incorrect since some high bits are discarded (are you missing some known bits analysis?).
Think of positive signed input value that has 1 somewhere in high 16 bits, e.g. 0x4000000000000000
'icmp select' sequence gives 32767 which is then truncated to i16 (no op in this case).
But cvt_pk takes low 16 bits from 0x40000000 and 0x00000000 and packs them into 32 bit value 0.
v_med3 with [[A]]=0 gives 0.
Does sdag have a similar combine?

16–17 ↗

(On Diff #313345)

For awareness: generic llvm-ir max and min intrinsics @llvm.umax. and @llvm.umin. were added recently.
It is expected to get llvm-ir combine that will transform cmp+select into intrinsic at some point and then there would be no need to match cmp+select in combines. You should test with min and max intrinsics for now. However since this is i64 I guess it is fine to use cmp + select since intrinsic min and max have to be lowered for s64.

[arsenm]

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

475

This is doing too much, this should just match the G_UMIN/G_UMAX/G_SMIN/G_SMAX opcodes. This shouldn't be responsible for matching the compare+select pattern

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

1 ↗

(On Diff #313345)

I would prefer if the test name had a more consistent name. How about combine-int-clamp.ll?

18–21 ↗

(On Diff #313345)

I dislike anonymous values in tests, can you add names here

[nhaehnle]

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

11 ↗

(On Diff #313345)

cvt_pk performs a clamp before packing into 16 bits. It's not very well documented.

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

1–2

I think this happened because of using clang-format.

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

11 ↗

(On Diff #313345)

The whole concept behind this, as Nicolai said, is, to have the two 32 bit-parts of the 64-bit number being clamped automatically against short boundaries by using v_cvt_pk_i16_i32. So the lower and higher 16 bits of the resulting 32 bit number should be clamped independently. If this fails for some case (very large numbers), the v_med3 instruction should make sure that the value actually gets clamped.

18–21 ↗

(On Diff #313345)

Hi, what do you mean by anonymous values in this context?

[tsymalla]

Mostly renamings.

[tsymalla]

Refactored the pattern matching algorithm. I removed the construct from MIPatternMatch and implemented it directly in the PostLegalizerCombiner.

[tsymalla]

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

475

I removed this construct. There area helpers already which could be extended for matching the G_*MAX/MIN opcodes. I tried to implement this straight forward directly in the matcher.

[arsenm]

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

18–21 ↗

(On Diff #313345)

The numbered %0, %1. They should have fixed string names instead. opt -S -instnamer will do this for you or you can manually %name them

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

2

This has too many -s then

285

This should still be trying to match against the min/max opcodes, not looking for compare and selects

[tsymalla]

Renamed the identifiers in the lit test.

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

2

I reverted this change.

285

So, if I understand this correctly:

• The select/compare pattern I'm trying to match here will be matched to G_MIN/G_MAX
• This seems to be not implemented yet or not working for my case (as far as I can see that)

I'm not sure what should be achieved here. Should there either be a "future-proof" implementation for matching min and max which is not working yet, an additional case for matching min and max which falls back to my current implementation or a combination of a) extension of the pattern matching that combines select and icmp to max / min and b) an implementation which only matches max / min and combines it accordingly?
Would you mind explaining this to me a little bit more?

llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll

18–21 ↗

(On Diff #313345)

Thanks. I manually named them.

[foad]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

15–16

You can use the git-clang-format script to avoid reformatting the whole file. If you put it on your path then you can invoke it as e.g. git clang-format @^ to only reformat the parts of the file that were changed by your most recent commit.

311–319

Use // even for block comments please.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

285

The compare+select pattern you are matching here should be matched separately into the min/max opcodes as a better, more canonical form. This is not implemented in GlobalISel yet.

Matching min/max should be implemented in GlobalISel, however I don't think this is a priority. Theoretically the IR optimizers should be matching these in the vast majority of cases with the new min/max intrinsics. A GlobalISel combine would theoretically be useful if the compare/select pattern were to be emitted by independent legalizations, but I'm currently not aware of any implemented case where this could happen

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

285

So, what would be the right approach to handle this in your opinion? As far as I understand it doesn't make sense to have an implementation for this specific case where a generic implementation would be more suitable. Would you say it makes more sense to wait for the max / min matching in GlobalISel and implement my combine around it?

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

285

You don't need to wait. You can just do it with the min/max opcodes now. You can either write MIR tests or change the IR tests to use the intrinsics already

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

311–319

Thanks, Jay. I will change this.

[tsymalla]

This moves the implementation to the PreLegalizer and is based on LLVM max/min intrinsics.

[arsenm]

I would expect this in the post legalizer combiner, and going directly from i64 is a bit weird. What does the post-legalized MIR look like for this?

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

455

Remove leftover whitespace change

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

191–192

Unrelated formatting changes

248

Extra whitespace change

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

62

Capitalize

74

Shouldn't construct single use MachineIRBuilder, there should be one in the combiner state

104

Capitalize

119

This is just noise, remove it

121

Don't construct fresh builder

129

You can just do Unmerge.getReg(0)

134

It would be better to use a generic operation pre-regbank select rather than forcing register classes here

148–149

Bad shadowing name

151–153

You can combine this as auto MinBoundaryDst = B.buildConstant(S32, min)

[tsymalla]

Previously this was implemented in the PostLegalizer. The SMAX / SMIN MIR gets lowered to a pattern of G_SELECT / G_ICMP, so the state after PostLegalizing is equivalent to what I've implemented in the tests before matching this to the min/max instructions. Disabling the optimization in the implementation here yields a result similar to the following:

%62:_(s1) = G_ICMP intpred(slt), %53:_(s32), %25:_
%63:_(s64) = G_CONSTANT i64 0
%28:_(s64) = G_SELECT %62:_(s1), %63:_, %61:_
%29:_(s64) = G_CONSTANT i64 32767
%37:_(s1) = G_ICMP intpred(slt), %28:_(s64), %29:_
%30:_(s64) = G_SELECT %37:_(s1), %28:_, %29:_
%31:_(s64) = G_CONSTANT i64 -32768
%36:_(s1) = G_ICMP intpred(sgt), %30:_(s64), %31:_
%32:_(s64) = G_SELECT %36:_(s1), %30:_, %31:_
%34:_(s32) = G_TRUNC %32:_(s64)

So this is why I moved the implementation to the PreLegalize step where the G_SMIN / G_SMAX IR still exists:

%30:_(s64) = G_SMIN %28:_, %29:_
%32:_(s64) = G_SMAX %30:_, %31:_
%33:_(s16) = G_TRUNC %32:_(s64)

I don't exactly understand why the min / max instructions get lowered to this pattern so for the moment, the only way to get this producing the code I wish seems to match the pattern in the PreLegalize step. By the way, the goal here is to check explicitly if an long should get clamped to short boundaries (or inbetween). Instead of producing some comparisons, this should yield these two instructions as I've figured out with Nicolai.

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

134

How would I go for this?

[tsymalla]

Various changes according to the code review. I don't know why the formatting changes in the PostLegalizer keep showing up as they are already reverted to HEAD...

[arsenm]

Do we do the equivalent combine in the DAG path? I don't recognize this one

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

71

Probably shouldn't print here especially when failure is likely. Ideally we would get a message from the generated combiner based on the defined combine name

132

I'd rather just repeat the VGPR_32RegClass (also shuoldn't use macro-like naming convention)

134

You can follow the example of the other G_AMDGPU_* pseudos (e.g. G_AMDGPU_CVT_UBYTE0). I'm a bit ambivalent on whether we should do this in this case, as there are still some unanswered questions about how regbankselect should work. However, so far we've been using pseudos like this

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-short-clamp.ll

113

Fix this whitespace error

[Petar.Avramovic]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

137–141

You can replace this block with B.buildInstr(CvtOpcode, {CvtDst}, {Hi32, Lo32}, MI.getFlags());

201

Why do you need LegalizerInfo? CombinerHelper::isLegalOrBeforeLegalizer relies on LI being nullptr before legalizer.

[tsymalla]

No, I doubt there is support for this in the other path.

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

71

I removed this message. There aren't any other debug outputs in the other Legalize combines, too.

134

Thanks. With some help I was able to get this working and learned something new. This should now use the new pseudo and select it into the corresponding machine instruction.

137–141

Thanks. I removed this for the other buildInstr, too.

201

Thanks. This is not needed here, so I removed it.

[tsymalla]

Implemented a GMIR instruction for the V_CVT opcode and did some changes to the implementation according to the code reviews.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

149

A pseudo here would also apply for the same reasons

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

149

I will implement this as soon as I'll get access back to my machine. Is there a reason why in some cases pseudos exist, in some cases not?

[tsymalla]

This adds an additional G_MED3_S32 GIR opcode and uses it in the PreLegalizerCombiner.

[tsymalla]

Added the new MED3 pseudo.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

146

Don't need the register classes. You can also hide the createRegister calls with the builder as well

149

If we need to change how the operation is selected based on the register banks, it definitely should use a regbankselectable pseudo with generic register operands. If there isn't a meaningful handling change depending on the register banks, there's less reason to use a pseudo.

[tsymalla]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

146

Thanks. I reworked this aspect of the implementation. Should be a bit more clean now.

149

Thanks. :)

[tsymalla]

Removes the generic createVirtualRegister invocations and the manual setting of the VGPR32 register class.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

142–143

You can truncate directly into the destination register, you don't need this copy

llvm/lib/Target/AMDGPU/SIInstructions.td

2596

No _S32, this is also available for s16 on some targets

[tsymalla]

Renamed the G_MED3 opcode and removed a superfluous copy.

[tsymalla]

Hi, it seems, I am not permitted to commit this patch. Could somebody please do the commit for me? Thanks!

[arsenm]

In D93708#2524681, @tsymalla wrote:

Hi, it seems, I am not permitted to commit this patch. Could somebody please do the commit for me? Thanks!

You have to request commit access:
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

[tsymalla]

Requested access. Thanks.

[arsenm]

I've tried applying this but getting some build errors. You also have some remaining trailing whitespace and extra whitespace changes

[tsymalla]

In D93708#2525719, @arsenm wrote:

I've tried applying this but getting some build errors. You also have some remaining trailing whitespace and extra whitespace changes

I will revert those whitespace changes, thanks.
Where did you apply this? Locally? For me, this worked with a recent LLVM version.

[tsymalla]

I investigated the problem and it seems, there were some includes missing in PostLegalizerCombiner as there were changes in terms of how the instructions are made available to the TU.
I will fix this and upload a new patch.

[tsymalla]

Reverted whitespace changes, added missing includes.

[eugenis]

Hi,

This change, or one of the related change has broken the msan bot.
http://lab.llvm.org:8011/#/builders/5/builds/4134

==91167==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x35d38b6 in AMDGPUPreLegalizerCombinerHelper::matchClampI64ToI16(llvm::MachineInstr&, llvm::MachineRegisterInfo&, llvm::MachineFunction&, AMDGPUPreLegalizerCombinerHelper::ClampI64ToI16MatchInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:92:17
    #1 0x35e7a91 in operator() /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/Target/AMDGPU/AMDGPUGenPreLegalizeGICombiner.inc:1227:38
    #2 0x35e7a91 in (anonymous namespace)::AMDGPUGenPreLegalizerCombinerHelper::tryCombineAll(llvm::GISelChangeObserver&, llvm::MachineInstr&, llvm::MachineIRBuilder&, llvm::CombinerHelper&) const /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/Target/AMDGPU/AMDGPUGenPreLegalizeGICombiner.inc:1226:14
    #3 0x35d6aed in (anonymous namespace)::AMDGPUPreLegalizerCombinerInfo::combine(llvm::GISelChangeObserver&, llvm::MachineInstr&, llvm::MachineIRBuilder&) const /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:197:17
    #4 0xae8a3c4 in llvm::Combiner::combineMachineInstrs(llvm::MachineFunction&, llvm::GISelCSEInfo*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/GlobalISel/Combiner.cpp:150:24
    #5 0x35d629f in (anonymous namespace)::AMDGPUPreLegalizerCombiner::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:268:12
    #6 0x71117ca in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:72:13
    #7 0x7f07777 in llvm::FPPassManager::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1435:27
    #8 0x63360b8 in RunPassOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:178:25
    #9 0x63360b8 in RunAllPassesOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:476:9
    #10 0x63360b8 in (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:541:18
    #11 0x7f08ee4 in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1550:27
    #12 0x7f08ee4 in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:541:44
    #13 0x27ee02a in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:684:8
    #14 0x27e7c41 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:385:22
    #15 0x7f1bd530d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #16 0x2767739 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/llc+0x2767739)

Also, there is something very wrong with the diff here. It just shows a bunch of merge conflict markers. Please make sure that your commits include the relevant headers like "Differential Revision", etc - it's very hard to track things otherwise.

[arsenm]

In D93708#2537793, @eugenis wrote:

Hi,

This change, or one of the related change has broken the msan bot.
http://lab.llvm.org:8011/#/builders/5/builds/4134

==91167==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x35d38b6 in AMDGPUPreLegalizerCombinerHelper::matchClampI64ToI16(llvm::MachineInstr&, llvm::MachineRegisterInfo&, llvm::MachineFunction&, AMDGPUPreLegalizerCombinerHelper::ClampI64ToI16MatchInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:92:17
    #1 0x35e7a91 in operator() /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/Target/AMDGPU/AMDGPUGenPreLegalizeGICombiner.inc:1227:38
    #2 0x35e7a91 in (anonymous namespace)::AMDGPUGenPreLegalizerCombinerHelper::tryCombineAll(llvm::GISelChangeObserver&, llvm::MachineInstr&, llvm::MachineIRBuilder&, llvm::CombinerHelper&) const /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/Target/AMDGPU/AMDGPUGenPreLegalizeGICombiner.inc:1226:14
    #3 0x35d6aed in (anonymous namespace)::AMDGPUPreLegalizerCombinerInfo::combine(llvm::GISelChangeObserver&, llvm::MachineInstr&, llvm::MachineIRBuilder&) const /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:197:17
    #4 0xae8a3c4 in llvm::Combiner::combineMachineInstrs(llvm::MachineFunction&, llvm::GISelCSEInfo*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/GlobalISel/Combiner.cpp:150:24
    #5 0x35d629f in (anonymous namespace)::AMDGPUPreLegalizerCombiner::runOnMachineFunction(llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp:268:12
    #6 0x71117ca in llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:72:13
    #7 0x7f07777 in llvm::FPPassManager::runOnFunction(llvm::Function&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1435:27
    #8 0x63360b8 in RunPassOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:178:25
    #9 0x63360b8 in RunAllPassesOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:476:9
    #10 0x63360b8 in (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:541:18
    #11 0x7f08ee4 in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1550:27
    #12 0x7f08ee4 in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:541:44
    #13 0x27ee02a in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:684:8
    #14 0x27e7c41 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:385:22
    #15 0x7f1bd530d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #16 0x2767739 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/llc+0x2767739)

Also, there is something very wrong with the diff here. It just shows a bunch of merge conflict markers. Please make sure that your commits include the relevant headers like "Differential Revision", etc - it's very hard to track things otherwise.

I think that's what D95878 is for

[sebastian-ne]

It may take some time until @tsymalla can look at the fix in D95878 again, so I reverted this for the time being.

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

   return BinaryOp_match<LHS, RHS, TargetOpcode::G_ASHR, false>(L, R);
 }
 
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, false>
+m_GSMax(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMAX, false>(L, R);
+}
+
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, false>
+m_GSMin(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_SMIN, false>(L, R);
+}
+
 // Helper for unary instructions (G_[ZSA]EXT/G_TRUNC) etc
 template <typename SrcTy, unsigned Opcode> struct UnaryOp_match {
   SrcTy L;

                          TargetOpcode::G_INSERT_VECTOR_ELT>(Src0, Src1, Src2);
 }
 
+template <typename Src0Ty, typename Src1Ty, typename Src2Ty>
+inline TernaryOp_match<Src0Ty, Src1Ty, Src2Ty, TargetOpcode::G_SELECT>
+m_GISelect(const Src0Ty &Src0, const Src1Ty &Src1, const Src2Ty &Src2) {
+  return TernaryOp_match<Src0Ty, Src1Ty, Src2Ty, TargetOpcode::G_SELECT>(
+      Src0, Src1, Src2);
+}
+
 /// Matches a register negated by a G_SUB.
 /// G_SUB 0, %negated_reg
 template <typename SrcTy>

   return m_GXor(Src, m_AllOnesInt());
 }
 
-} // namespace GMIPatternMatch
+
+} // namespace MIPatternMatch
 } // namespace llvm
 
 #endif

llvm/lib/Target/AMDGPU/AMDGPUCombine.td

          [{ return PostLegalizerHelper.matchCvtF32UByteN(*${cvt_f32_ubyteN}, ${matchinfo}); }]),
   (apply [{ PostLegalizerHelper.applyCvtF32UByteN(*${cvt_f32_ubyteN}, ${matchinfo}); }])>;
 
+def clamp_i64_to_i16_matchdata : GIDefMatchData<"AMDGPUPreLegalizerCombinerHelper::ClampI64ToI16MatchInfo">;
+
+def clamp_i64_to_i16 : GICombineRule<
+  (defs root:$clamp_i64_to_i16, clamp_i64_to_i16_matchdata:$matchinfo),
+  (match (wip_match_opcode G_TRUNC):$clamp_i64_to_i16,
+      [{ return PreLegalizerHelper.matchClampI64ToI16(*${clamp_i64_to_i16}, MRI, *MF, ${matchinfo}); }]),
+  (apply [{ PreLegalizerHelper.applyClampI64ToI16(*${clamp_i64_to_i16}, ${matchinfo}); }])>;
+
 // Combines which should only apply on SI/VI
 def gfx6gfx7_combines : GICombineGroup<[fcmp_select_to_fmin_fmax_legacy]>;
 
-
 def AMDGPUPreLegalizerCombinerHelper: GICombinerHelper<
-  "AMDGPUGenPreLegalizerCombinerHelper", [all_combines]> {
+  "AMDGPUGenPreLegalizerCombinerHelper", [all_combines, clamp_i64_to_i16]> {
   let DisableRuleOption = "amdgpuprelegalizercombiner-disable-rule";
+  let StateClass = "AMDGPUPreLegalizerCombinerHelperState";
 }
 
 def AMDGPUPostLegalizerCombinerHelper: GICombinerHelper<

llvm/lib/Target/AMDGPU/AMDGPUGISel.td

 def : GINodeEquiv<G_AMDGPU_CVT_F32_UBYTE2, AMDGPUcvt_f32_ubyte2>;
 def : GINodeEquiv<G_AMDGPU_CVT_F32_UBYTE3, AMDGPUcvt_f32_ubyte3>;
 
+def : GINodeEquiv<G_AMDGPU_CVT_PK_I16_I32, AMDGPUpk_i16_i32_impl>;
+def : GINodeEquiv<G_AMDGPU_MED3_S32, AMDGPUsmed3>;
+
 def : GINodeEquiv<G_AMDGPU_ATOMIC_CMPXCHG, AMDGPUatomic_cmp_swap>;
 def : GINodeEquiv<G_AMDGPU_BUFFER_LOAD, SIbuffer_load>;
 def : GINodeEquiv<G_AMDGPU_BUFFER_LOAD_USHORT, SIbuffer_load_ushort>;

llvm/lib/Target/AMDGPU/AMDGPUInstrInfo.td

 def AMDGPUcvt_f32_ubyte3 : SDNode<"AMDGPUISD::CVT_F32_UBYTE3",
   SDTIntToFPOp, []>;
 
+def AMDGPUcvt_pk_i16_i32 : SDNode<"AMDGPUISD::CVT_PK_I16_I32",
+  AMDGPUIntPackOp, []>;
 
 // urecip - This operation is a helper for integer division, it returns the
 // result of 1 / a as a fractional unsigned integer.

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp

 
 #include "AMDGPUTargetMachine.h"
 #include "AMDGPULegalizerInfo.h"
+
+
 #include "llvm/CodeGen/GlobalISel/Combiner.h"
 #include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
 #include "llvm/CodeGen/GlobalISel/CombinerInfo.h"

   bool runOnMachineFunction(MachineFunction &MF) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;
+
 private:
   bool IsOptNone;
 };

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

 using namespace llvm;
 using namespace MIPatternMatch;
 
+class AMDGPUPreLegalizerCombinerHelper {
+protected:
+  MachineIRBuilder &B;
+  MachineFunction &MF;
+  MachineRegisterInfo &MRI;
+  CombinerHelper &Helper;
+
+public:
+  AMDGPUPreLegalizerCombinerHelper(MachineIRBuilder &B, CombinerHelper &Helper)
+      : B(B), MF(B.getMF()), MRI(*B.getMRI()), Helper(Helper){};
+
+  struct ClampI64ToI16MatchInfo {
+    int64_t Cmp1;
+    int64_t Cmp2;
+    Register Origin;
+  };
+
+  bool matchClampI64ToI16(MachineInstr &MI, MachineRegisterInfo &MRI,
+                          MachineFunction &MF,
+                          ClampI64ToI16MatchInfo &MatchInfo);
+
+  void applyClampI64ToI16(MachineInstr &MI,
+                          const ClampI64ToI16MatchInfo &MatchInfo);
+};
+
+bool AMDGPUPreLegalizerCombinerHelper::matchClampI64ToI16(
+    MachineInstr &MI, MachineRegisterInfo &MRI, MachineFunction &MF,
+    ClampI64ToI16MatchInfo &MatchInfo) {
+  assert(MI.getOpcode() == TargetOpcode::G_TRUNC && "Invalid instruction!");
+
+  // Try to find a pattern where an i64 value should get clamped to short.
+  const LLT SrcType = MRI.getType(MI.getOperand(1).getReg());
+  if (SrcType != LLT::scalar(64))
+    return false;
+
+  const LLT DstType = MRI.getType(MI.getOperand(0).getReg());
+  if (DstType != LLT::scalar(16))
+    return false;
+
+  Register Base;
+
+  // Try to match a combination of min / max MIR opcodes.
+  if (mi_match(MI.getOperand(1).getReg(), MRI, m_GSMin(m_Reg(Base), m_ICst(MatchInfo.Cmp1)))) {
+    if (!mi_match(Base, MRI, m_GSMax(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)))) {
+      return false;
+    }
+  }
+
+  if (mi_match(MI.getOperand(1).getReg(), MRI, m_GSMax(m_Reg(Base), m_ICst(MatchInfo.Cmp1)))) {
+    if (!mi_match(Base, MRI, m_GSMin(m_Reg(MatchInfo.Origin), m_ICst(MatchInfo.Cmp2)))) {
+      return false;
+    }
+  }
+   
+  const auto Cmp1 = MatchInfo.Cmp1;
+  const auto Cmp2 = MatchInfo.Cmp2;
+  const auto Diff = std::abs(Cmp2 - Cmp1);
+
+  // If the difference between both comparison values is 0 or 1, there is no
+  // need to clamp.
+  if (Diff == 0 || Diff == 1)
+    return false;
+
+  const int64_t Min = std::numeric_limits<int16_t>::min();
+  const int64_t Max = std::numeric_limits<int16_t>::max();
+
+  // Check if the comparison values are between SHORT_MIN and SHORT_MAX.
+  return ((Cmp2 >= Cmp1 && Cmp1 >= Min && Cmp2 <= Max) ||
+          (Cmp1 >= Cmp2 && Cmp1 <= Max && Cmp2 >= Min));
+}
+
+// We want to find a combination of instructions that
+// gets generated when an i64 gets clamped to i16.
+// The corresponding pattern is:
+// G_MAX / G_MAX for i16 <= G_TRUNC i64.
+// This can be efficiently written as following:
+// v_cvt_pk_i16_i32 v0, v0, v1
+// v_med3_i32 v0, Clamp_Min, v0, Clamp_Max
+void AMDGPUPreLegalizerCombinerHelper::applyClampI64ToI16(
+    MachineInstr &MI, const ClampI64ToI16MatchInfo &MatchInfo) {
+  MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+
+  Register Src = MatchInfo.Origin;
+  assert(MRI.getType(Src) == LLT::scalar(64));
+  const LLT S32 = LLT::scalar(32);
+
+  B.setMBB(*MI.getParent());
+  B.setInstrAndDebugLoc(MI);
+
+  auto Unmerge = B.buildUnmerge(S32, Src);
+  Register Hi32 = Unmerge.getReg(0);
+  Register Lo32 = Unmerge.getReg(1);
+  MRI.setRegClass(Hi32, &AMDGPU::VGPR_32RegClass);
+  MRI.setRegClass(Lo32, &AMDGPU::VGPR_32RegClass);
+
+  assert(MI.getOpcode() != AMDGPU::G_AMDGPU_CVT_PK_I16_I32);
+
+  Register CvtDst = MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
+  const LLT V2S16 = LLT::vector(2, 16);
+  MRI.setType(CvtDst, V2S16);
+
+  B.buildInstr(AMDGPU::G_AMDGPU_CVT_PK_I16_I32,
+    {CvtDst},
+    {Hi32, Lo32},
+    MI.getFlags());
+
+  auto MinBoundary = std::min(MatchInfo.Cmp1, MatchInfo.Cmp2);
+  auto MaxBoundary = std::max(MatchInfo.Cmp1, MatchInfo.Cmp2);
+
+  auto MinBoundaryDst = B.buildConstant(S32, MinBoundary);
+  MRI.setRegClass(MinBoundaryDst.getReg(0), &AMDGPU::VGPR_32RegClass);
+
+  auto MaxBoundaryDst = B.buildConstant(S32, MaxBoundary);
+  MRI.setRegClass(MaxBoundaryDst.getReg(0), &AMDGPU::VGPR_32RegClass);
+
+  Register MedDst = MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
+  MRI.setType(MedDst, S32);
+
+  Register CvtDst32 = MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
+  MRI.setType(CvtDst32, S32);
+
+  B.buildBitcast(CvtDst32, CvtDst);
+
+  B.buildInstr(AMDGPU::G_AMDGPU_MED3_S32,
+    {MedDst},
+    {MinBoundaryDst.getReg(0), CvtDst32, MaxBoundaryDst.getReg(0)},
+    MI.getFlags());
+  
+  Register TruncDst = MRI.createGenericVirtualRegister(LLT::scalar(16));
+  B.buildTrunc(TruncDst, MedDst);
+  B.buildCopy(MI.getOperand(0).getReg(), TruncDst);
+
+  MI.eraseFromParent();
+}
+
+class AMDGPUPreLegalizerCombinerHelperState {
+protected:
+  CombinerHelper &Helper;
+  AMDGPUPreLegalizerCombinerHelper &PreLegalizerHelper;
+
+public:
+  AMDGPUPreLegalizerCombinerHelperState(
+      CombinerHelper &Helper,
+      AMDGPUPreLegalizerCombinerHelper &PreLegalizerHelper)
+      : Helper(Helper), PreLegalizerHelper(PreLegalizerHelper) {}
+};
+
 #define AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
 #include "AMDGPUGenPreLegalizeGICombiner.inc"
 #undef AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS

                                               MachineInstr &MI,
                                               MachineIRBuilder &B) const {
   CombinerHelper Helper(Observer, B, KB, MDT);
-  AMDGPUGenPreLegalizerCombinerHelper Generated(GeneratedRuleCfg);
+  AMDGPUPreLegalizerCombinerHelper PreLegalizerHelper(B, Helper);
+  AMDGPUGenPreLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper,
+                                                 PreLegalizerHelper);
 
   if (Generated.tryCombineAll(Observer, MI, B, Helper))
     return true;

   const Function &F = MF.getFunction();
   bool EnableOpt =
       MF.getTarget().getOptLevel() != CodeGenOpt::None && !skipFunction(F);
+
   GISelKnownBits *KB = &getAnalysis<GISelKnownBitsAnalysis>().get(MF);
   MachineDominatorTree *MDT =
       IsOptNone ? nullptr : &getAnalysis<MachineDominatorTree>();

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

   case AMDGPU::G_AMDGPU_CVT_F32_UBYTE1:
   case AMDGPU::G_AMDGPU_CVT_F32_UBYTE2:
   case AMDGPU::G_AMDGPU_CVT_F32_UBYTE3:
+  case AMDGPU::G_AMDGPU_CVT_PK_I16_I32:
+  case AMDGPU::G_AMDGPU_MED3_S32:
     return getDefaultMappingVOP(MI);
   case AMDGPU::G_UMULH:
   case AMDGPU::G_SMULH: {

llvm/lib/Target/AMDGPU/SIInstructions.td

 }
 }
 
+def G_AMDGPU_CVT_PK_I16_I32 : AMDGPUGenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type0:$src0, type0:$src1);
+  let hasSideEffects = 0;
+}
+
+def G_AMDGPU_MED3_S32 : AMDGPUGenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type0:$src0, type0:$src1, type0:$src2);
+  let hasSideEffects = 0;
+}
+
 // Atomic cmpxchg. $cmpval ad $newval are packed in a single vector
 // operand Expects a MachineMemOperand in addition to explicit
 // operands.

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-short-clamp.ll

+; RUN: llc -global-isel -mcpu=tahiti -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck --check-prefixes=GFX678,GFX6789 %s
+; RUN: llc -global-isel -mcpu=gfx900 -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck --check-prefixes=GFX9,GFX6789 %s
+; RUN: llc -global-isel -mcpu=gfx1010 -march=amdgcn -verify-machineinstrs < %s | FileCheck --check-prefix=GFX10 %s
+
+declare i64 @llvm.smax.i64(i64, i64)
+declare i64 @llvm.smin.i64(i64, i64)
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16
+; GFX678: v_cvt_pk_i16_i32_e32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX9: v_cvt_pk_i16_i32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX6789: v_mov_b32_e32 [[B]], 0x7fff
+; GFX6789: v_mov_b32_e32 [[C:v[0-9]+]], 0xffff8000
+; GFX6789: v_med3_i32 [[A]], [[C]], [[A]], [[B]]
+; GFX10: v_cvt_pk_i16_i32_e64 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX10: v_mov_b32_e32 [[C:v[0-9]+]], 0xffff8000
+; GFX10: v_med3_i32 [[A]], [[C]], [[A]], 0x7fff
+define i16 @v_clamp_i64_i16(i64 %in) #0 {
+entry:
+  %max = call i64 @llvm.smax.i64(i64 %in, i64 -32768)
+  %min = call i64 @llvm.smin.i64(i64 %max, i64 32767)
+  %result = trunc i64 %min to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_reverse
+; GFX678: v_cvt_pk_i16_i32_e32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX9: v_cvt_pk_i16_i32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX6789: v_mov_b32_e32 [[B]], 0x7fff
+; GFX6789: v_mov_b32_e32 [[C:v[0-9]+]], 0xffff8000 
+; GFX6789: v_med3_i32 [[A]], [[C]], [[A]], [[B]]
+; GFX10: v_cvt_pk_i16_i32_e64 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX10: v_mov_b32_e32 [[C:v[0-9]+]], 0xffff8000
+; GFX10: v_med3_i32 [[A]], [[C]], [[A]], 0x7fff
+define i16 @v_clamp_i64_i16_reverse(i64 %in) #0 {
+entry:
+  %min = call i64 @llvm.smin.i64(i64 %in, i64 32767)
+  %max = call i64 @llvm.smax.i64(i64 %min, i64 -32768)
+  %result = trunc i64 %max to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_invalid_lower
+; GFX6789: v_mov_b32_e32 [[B:v[0-9]+]], 0x8001
+; GFX6789: v_cndmask_b32_e32 [[A:v[0-9]+]], [[B]], [[A]], vcc
+; GFX6789: v_cndmask_b32_e32 [[C:v[0-9]+]], 0, [[C]], vcc
+
+; GFX10: v_cndmask_b32_e32 [[A:v[0-9]+]], 0x8001, [[A]], vcc_lo
+; GFX10: v_cndmask_b32_e32 [[B:v[0-9]+]], 0, [[B]], vcc_lo
+define i16 @v_clamp_i64_i16_invalid_lower(i64 %in) #0 {
+entry:
+  %min = call i64 @llvm.smin.i64(i64 %in, i64 32769)
+  %max = call i64 @llvm.smax.i64(i64 %min, i64 -32768)
+  %result = trunc i64 %max to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_invalid_lower_and_higher
+; GFX6789: v_mov_b32_e32 [[B:v[0-9]+]], 0x8000
+; GFX6789: v_cndmask_b32_e32 [[A:v[0-9]+]], [[B]], [[A]], vcc
+; GFX10: v_cndmask_b32_e32 [[A:v[0-9]+]], 0x8000, [[A]], vcc_lo
+define i16 @v_clamp_i64_i16_invalid_lower_and_higher(i64 %in) #0 {
+entry:
+  %max = call i64 @llvm.smax.i64(i64 %in, i64 -32769)
+  %min = call i64 @llvm.smin.i64(i64 %max, i64 32768)
+  %result = trunc i64 %min to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_lower_than_short
+; GFX678: v_cvt_pk_i16_i32_e32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX9: v_cvt_pk_i16_i32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX6789: v_mov_b32_e32 [[B]], 0x100
+; GFX6789: v_mov_b32_e32 [[C:v[0-9]+]], 0xffffff01
+; GFX6789: v_med3_i32 [[A]], [[C]], [[A]], [[B]]
+; GFX10: v_cvt_pk_i16_i32_e64 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX10: v_mov_b32_e32 [[C:v[0-9]+]], 0xffffff01
+; GFX10: v_med3_i32 [[A]], [[C]], [[A]], 0x100
+define i16 @v_clamp_i64_i16_lower_than_short(i64 %in) #0 {
+entry:
+  %min = call i64 @llvm.smin.i64(i64 %in, i64 256)
+  %max = call i64 @llvm.smax.i64(i64 %min, i64 -255)
+  %result = trunc i64 %max to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_lower_than_short_reverse
+; GFX678: v_cvt_pk_i16_i32_e32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX9: v_cvt_pk_i16_i32 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX6789: v_mov_b32_e32 [[B]], 0x100
+; GFX6789: v_mov_b32_e32 [[C:v[0-9]+]], 0xffffff01
+; GFX6789: v_med3_i32 [[A]], [[C]], [[A]], [[B]]
+; GFX10: v_cvt_pk_i16_i32_e64 [[A:v[0-9]+]], [[A]], [[B:v[0-9]+]]
+; GFX10: v_mov_b32_e32 [[C:v[0-9]+]], 0xffffff01
+; GFX10: v_med3_i32 [[A]], [[C]], [[A]], 0x100
+define i16 @v_clamp_i64_i16_lower_than_short_reverse(i64 %in) #0 {
+entry:
+  %max = call i64 @llvm.smax.i64(i64 %in, i64 -255)
+  %min = call i64 @llvm.smin.i64(i64 %max, i64 256)
+  %result = trunc i64 %min to i16
+  ret i16 %result
+}
+
+; GFX10-LABEL: {{^}}v_clamp_i64_i16_zero
+; GFX6789: v_mov_b32_e32 v0, 0
+; GFX10: v_mov_b32_e32 v0, 0
+define i16 @v_clamp_i64_i16_zero(i64 %in) #0 {
+entry:
+  %max = call i64 @llvm.smax.i64(i64 %in, i64 0)
+  %min = call i64 @llvm.smin.i64(i64 %max, i64 0)
+  %result = trunc i64 %min to i16
+  ret i16 %result
+}