This is an archive of the discontinued LLVM Phabricator instance.

Differential D57539

[InstCombine] Extra null-checking on TFE/LWE support
ClosedPublic

Authored by hliao on Jan 31 2019, 1:03 PM.

Details

Reviewers
yaxunl
dstuttard
msearles
arsenm
Commits
rG8b323f53eb40: [InstCombine] Extra null-checking on TFE/LWE support
rL352904: [InstCombine] Extra null-checking on TFE/LWE support
Summary

Need extra null-checking after ConstantInt dyn_cast.

Diff Detail

Event Timeline

hliao created this revision.Jan 31 2019, 1:03 PM
Herald added subscribers: llvm-commits, hiraditya. · View Herald TranscriptJan 31 2019, 1:03 PM
lebedev.ri added a subscriber: lebedev.ri.Jan 31 2019, 1:05 PM
  1. -U99999
  2. Test?
hliao added a comment.Jan 31 2019, 1:08 PM

this's early review. test will be added soon.

msearles added a subscriber: msearles.Feb 1 2019, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 8:24 AM
hliao updated this revision to Diff 184771.Feb 1 2019, 9:21 AM

test case is added

arsenm added a subscriber: arsenm.Feb 1 2019, 9:26 AM
arsenm added inline comments.
llvm/test/Transforms/InstCombine/crash.tfe.ll
1 ↗(On Diff #184771)

This is going to fail if AMDGPU isn't built. This should go in test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts.ll with the others

hliao updated this revision to Diff 184776.Feb 1 2019, 9:44 AM

move to AMDGPU specific test

Herald added subscribers: nhaehnle, jvesely. · View Herald TranscriptFeb 1 2019, 9:44 AM
hliao added reviewers: msearles, arsenm.Feb 1 2019, 9:59 AM
Herald added a subscriber: wdng. · View Herald TranscriptFeb 1 2019, 9:59 AM
hliao updated this revision to Diff 184795.Feb 1 2019, 11:07 AM

update with -U9999

arsenm added a comment.Feb 1 2019, 11:07 AM
In D57539#1380953, @hliao wrote:

move to AMDGPU specific test

I meant literally in the same file. Can you move it in and follow the same naming scheme as the other functions there?

kzhuravl added a subscriber: kzhuravl.Feb 1 2019, 11:13 AM
hliao updated this revision to Diff 184800.Feb 1 2019, 11:18 AM

merge that trivial test case into the originally one.

arsenm accepted this revision.Feb 1 2019, 11:25 AM

LGTM

This revision is now accepted and ready to land.Feb 1 2019, 11:25 AM
Closed by commit rL352904: [InstCombine] Extra null-checking on TFE/LWE support (authored by hliao). · Explain WhyFeb 1 2019, 11:53 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Transforms/
InstCombine/
7 lines
test/
Transforms/
InstCombine/
AMDGPU/
7 lines

Diff 184776

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

Context not available.
return nullptr; return nullptr;
// Need to change to new instruction format // Need to change to new instruction format
ConstantInt *TFC = nullptr;
bool TFELWEEnabled = false; bool TFELWEEnabled = false;
if (TFCIdx > 0) { if (TFCIdx > 0) {
TFC = dyn_cast<ConstantInt>(II->getArgOperand(TFCIdx)); if (ConstantInt *TFC = dyn_cast<ConstantInt>(II->getArgOperand(TFCIdx)))
TFELWEEnabled = TFC->getZExtValue() & 0x1 // TFE TFELWEEnabled = TFC->getZExtValue() & 0x1 // TFE
|| TFC->getZExtValue() & 0x2; // LWE || TFC->getZExtValue() & 0x2; // LWE
} }
if (TFELWEEnabled) if (TFELWEEnabled)
Context not available.

llvm/test/Transforms/InstCombine/AMDGPU/crash.tfe.ll

  • This file was added.
; RUN: opt -instcombine -S -mtriple=amdgcn-amd-amdhsa -o - %s
define protected <4 x half> @__llvm_amdgcn_image_sample_d_1darray_v4f16_f32_f32(i32, float, float, float, float, <8 x i32>, <4 x i32>, i1 zeroext, i32, i32) local_unnamed_addr {
%tmp = tail call <4 x half> @llvm.amdgcn.image.sample.d.1darray.v4f16.f32.f32(i32 %0, float %1, float %2, float %3, float %4, <8 x i32> %5, <4 x i32> %6, i1 zeroext %7, i32 %8, i32 %9) #1
ret <4 x half> %tmp
}
declare <4 x half> @llvm.amdgcn.image.sample.d.1darray.v4f16.f32.f32(i32, float, float, float, float, <8 x i32>, <4 x i32>, i1, i32, i32)