Skip to content

Commit 3b09937

Browse files
committed
AMDGPU: Fix treating unknown mem operands as uniform (#168980)
The test changes are mostly GlobalISel specific regressions. GlobalISel is still relying on isUniformMMO, but it doesn't really have an excuse for doing so. These should be avoidable with new regbankselect. There is an additional regression for addrspacecast for cov4. We probably ought to be using a separate PseudoSourceValue for the access of the queue pointer.
1 parent 9ce0c07 commit 3b09937

File tree

5 files changed

+277
-95
lines changed

5 files changed

+277
-95
lines changed

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4451,16 +4451,14 @@ bool AMDGPUDAGToDAGISel::isUniformLoad(const SDNode *N) const {
44514451
const auto *Ld = cast<LoadSDNode>(N);
44524452
const MachineMemOperand *MMO = Ld->getMemOperand();
44534453

4454-
if (Ld->isDivergent()) {
4455-
// FIXME: We ought to able able to take the direct isDivergent result. We
4456-
// cannot rely on the MMO for a uniformity check, and should stop using
4457-
// it. This is a hack for 2 ways that the IR divergence analysis is superior
4458-
// to the DAG divergence: Recognizing shift-of-workitem-id as always
4459-
// uniform, and isSingleLaneExecution. These should be handled in the DAG
4460-
// version, and then this can be dropped.
4461-
if (!MMO->getValue() || !AMDGPU::isUniformMMO(MMO))
4462-
return false;
4463-
}
4454+
// FIXME: We ought to able able to take the direct isDivergent result. We
4455+
// cannot rely on the MMO for a uniformity check, and should stop using
4456+
// it. This is a hack for 2 ways that the IR divergence analysis is superior
4457+
// to the DAG divergence: Recognizing shift-of-workitem-id as always
4458+
// uniform, and isSingleLaneExecution. These should be handled in the DAG
4459+
// version, and then this can be dropped.
4460+
if (Ld->isDivergent() && !AMDGPU::isUniformMMO(MMO))
4461+
return false;
44644462

44654463
return MMO->getSize().hasValue() &&
44664464
Ld->getAlign() >=

llvm/lib/Target/AMDGPU/AMDGPUInstrInfo.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,13 @@ bool AMDGPU::isUniformMMO(const MachineMemOperand *MMO) {
3535
PSV->isJumpTable();
3636
}
3737

38-
// FIXME: null value is should be treated as unknown, not as uniform.
39-
return true;
38+
// Unknown value.
39+
return false;
4040
}
4141

4242
// UndefValue means this is a load of a kernel input. These are uniform.
4343
// Sometimes LDS instructions have constant pointers.
44-
// If Ptr is null, then that means this mem operand contains a
45-
// PseudoSourceValue like GOT.
46-
if (!Ptr || isa<UndefValue, Constant, GlobalValue>(Ptr))
44+
if (isa<UndefValue, Constant, GlobalValue>(Ptr))
4745
return true;
4846

4947
if (MMO->getAddrSpace() == AMDGPUAS::CONSTANT_ADDRESS_32BIT)

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,7 @@ Register AMDGPULegalizerInfo::getSegmentAperture(
23622362
if (!loadInputValue(QueuePtr, B, AMDGPUFunctionArgInfo::QUEUE_PTR))
23632363
return Register();
23642364

2365-
// TODO: can we be smarter about machine pointer info?
2365+
// TODO: Use custom PseudoSourceValue
23662366
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
23672367

23682368
// Offset into amd_queue_t for group_segment_aperture_base_hi /

0 commit comments

Comments
 (0)