Skip to content

Commit

Permalink
[AMDGPU] Fix for AMDGPU MUL_I24 known bits calculation
Browse files Browse the repository at this point in the history
At present, the code calculating known bits of AMDGPU MUL_I24 confuses
the concepts of "non-negative number" and "positive number".

In some situations, it results in incorrect code. I have a case where
the optimizer replaces the result of calculating MUL_I24(-5, 0) with -8.

Differential Revision: https://reviews.llvm.org/D70367

NB: patch was authored by ekuznetsov139; this patch is submitted prior to
the submission of the patch to llvm-trunk; there is agreement on the fix;
the lit test is still under revision. The patch is submitted to amd-stg-open-hcc
to resolve several high-priority internal bugs. Lastly, the lit test was tweaked
to fix a syntax error not yet corrected in the phab patch.

Change-Id: Iaced35ee4b97946ebb12515f08ea39b9bf5c48ca
  • Loading branch information
searlmc1 committed Dec 7, 2019
1 parent 50c09ca commit 4075636
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
3 changes: 3 additions & 0 deletions llvm/include/llvm/Support/KnownBits.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ struct KnownBits {
/// Returns true if this value is known to be non-negative.
bool isNonNegative() const { return Zero.isSignBitSet(); }

/// Returns true if this value is known to be positive.
bool isStrictlyPositive() const { return Zero.isSignBitSet() && !One.isNullValue(); }

/// Make this value negative.
void makeNegative() {
One.setSignBit();
Expand Down
17 changes: 8 additions & 9 deletions llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4440,24 +4440,23 @@ void AMDGPUTargetLowering::computeKnownBitsForTargetNode(
LHSKnown = LHSKnown.trunc(24);
RHSKnown = RHSKnown.trunc(24);

bool Negative = false;
if (Opc == AMDGPUISD::MUL_I24) {
unsigned LHSValBits = 24 - LHSKnown.countMinSignBits();
unsigned RHSValBits = 24 - RHSKnown.countMinSignBits();
unsigned MaxValBits = std::min(LHSValBits + RHSValBits, 32u);
if (MaxValBits >= 32)
break;
bool LHSNegative = LHSKnown.isNegative();
bool LHSPositive = LHSKnown.isNonNegative();
bool LHSNonNegative = LHSKnown.isNonNegative();
bool LHSPositive = LHSKnown.isStrictlyPositive();
bool RHSNegative = RHSKnown.isNegative();
bool RHSPositive = RHSKnown.isNonNegative();
if ((!LHSNegative && !LHSPositive) || (!RHSNegative && !RHSPositive))
break;
Negative = (LHSNegative && RHSPositive) || (LHSPositive && RHSNegative);
if (Negative)
Known.One.setHighBits(32 - MaxValBits);
else
bool RHSNonNegative = RHSKnown.isNonNegative();
bool RHSPositive = RHSKnown.isStrictlyPositive();

if((LHSNonNegative && RHSNonNegative) || (LHSNegative && RHSNegative))
Known.Zero.setHighBits(32 - MaxValBits);
else if((LHSNegative && RHSPositive) || (LHSPositive && RHSNegative))
Known.One.setHighBits(32 - MaxValBits);
} else {
unsigned LHSValBits = 24 - LHSKnown.countMinLeadingZeros();
unsigned RHSValBits = 24 - RHSKnown.countMinLeadingZeros();
Expand Down
16 changes: 16 additions & 0 deletions llvm/test/CodeGen/AMDGPU/amdgpu-mul24-knownbits.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
; RUN: llc -mtriple amdgcn-amd-amdhsa -mcpu=gfx900 < %s | FileCheck --check-prefix=GCN %s
; GCN: mul
define weak_odr amdgpu_kernel void @test_mul24_knownbits_kernel(float addrspace(1)* %p) {
entry:
%0 = tail call i32 @llvm.amdgcn.workitem.id.x() #28
%tid = and i32 %0, 3
%1 = mul nsw i32 %tid, -5
%v1 = and i32 %1, -32
%v2 = sext i32 %v1 to i64
%v3 = getelementptr inbounds float, float addrspace(1)* %p, i64 %v2
store float 0.000, float addrspace(1)* %v3, align 4
ret void
}

; Function Attrs: nounwind readnone speculatable
declare i32 @llvm.amdgcn.workitem.id.x() #20

0 comments on commit 4075636

Please sign in to comment.