Skip to content

Commit cc3f048

Browse files
authored
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 47436ab commit cc3f048

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
@@ -4422,16 +4422,14 @@ bool AMDGPUDAGToDAGISel::isUniformLoad(const SDNode *N) const {
44224422
const auto *Ld = cast<LoadSDNode>(N);
44234423
const MachineMemOperand *MMO = Ld->getMemOperand();
44244424

4425-
if (Ld->isDivergent()) {
4426-
// FIXME: We ought to able able to take the direct isDivergent result. We
4427-
// cannot rely on the MMO for a uniformity check, and should stop using
4428-
// it. This is a hack for 2 ways that the IR divergence analysis is superior
4429-
// to the DAG divergence: Recognizing shift-of-workitem-id as always
4430-
// uniform, and isSingleLaneExecution. These should be handled in the DAG
4431-
// version, and then this can be dropped.
4432-
if (!MMO->getValue() || !AMDGPU::isUniformMMO(MMO))
4433-
return false;
4434-
}
4425+
// FIXME: We ought to able able to take the direct isDivergent result. We
4426+
// cannot rely on the MMO for a uniformity check, and should stop using
4427+
// it. This is a hack for 2 ways that the IR divergence analysis is superior
4428+
// to the DAG divergence: Recognizing shift-of-workitem-id as always
4429+
// uniform, and isSingleLaneExecution. These should be handled in the DAG
4430+
// version, and then this can be dropped.
4431+
if (Ld->isDivergent() && !AMDGPU::isUniformMMO(MMO))
4432+
return false;
44354433

44364434
return MMO->getSize().hasValue() &&
44374435
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)