shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[SeparateConstOffsetFromGEP] Avoid miscompiles related to trunc nuw/nsw (#154582)

Browse Source 
Drop poison generating flags on trunc when distributing trunc over
add/sub/or. We need to do this since for example
(add (trunc nuw A), (trunc nuw B)) is more poisonous than
(trunc nuw (add A, B))).

In some situations it is pessimistic to drop the flags. Such as
if the add in the example above also has the nuw flag. For now we
keep it simple and always drop the flags.

Worth mentioning is that we drop the flags when cloning
instructions and rebuilding the chain. This is done after the
"allowsPreservingNUW" checks in ConstantOffsetExtractor::Extract.
So we still take the "trunc nuw" into consideration when determining
if nuw can be preserved in the gep (which should be ok since that
check also require that all the involved binary operations has nuw).

Fixes #154116
This commit is contained in:
Bjorn Pettersson 2025-08-20 19:26:11 +02:00
parent 4ff7ac2330
commit 2d3167f8d8
No known key found for this signature in database
GPG Key ID: 07E53FDAB1AFBA1F
3 changed files with 12 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
View File

@ -642,6 +642,13 @@ Value *ConstantOffsetExtractor::applyExts(Value *V) {
Instruction *Ext = I->clone();
Ext->setOperand(0, Current);
// In ConstantOffsetExtractor::find we do not analyze nuw/nsw for trunc, so
// we assume that it is ok to redistribute trunc over add/sub/or. But for
// example (add (trunc nuw A), (trunc nuw B)) is more poisonous than (trunc
// nuw (add A, B))). To make such redistributions legal we drop all the
// poison generating flags from cloned trunc instructions here.
if (isa<TruncInst>(Ext))
Ext->dropPoisonGeneratingFlags();
Ext->insertBefore(*IP->getParent(), IP);
Current = Ext;
}

2
llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/preserve-inbounds.ll
View File

@ -311,7 +311,7 @@ entry:
define ptr @nuw_inbounds_implies_nuw_inbounds_trunc_nuw(ptr %p, i128 %i) {
; CHECK-LABEL: @nuw_inbounds_implies_nuw_inbounds_trunc_nuw(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = trunc nuw i128 [[I:%.*]] to i64
; CHECK-NEXT: [[TMP0:%.*]] = trunc i128 [[I:%.*]] to i64
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr inbounds nuw i32, ptr [[P:%.*]], i64 [[TMP0]]
; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr inbounds nuw i8, ptr [[TMP1]], i64 4
; CHECK-NEXT: ret ptr [[ARRAYIDX2]]

12
llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/rebuild-trunc.ll
View File

@ -1,13 +1,11 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1200 -passes=separate-const-offset-from-gep -S | FileCheck %s
; FIXME: (add (trunc nuw A), (trunc nuw B)) is more poisonous than
; (trunc nuw (add A, B))), so we should for example drop the
; nuw on the trunc when doing the rewrite here.
; Verify that we drop "nuw" from trunc.
define ptr @pr154116_nuw(ptr %p, i128 %i) {
; CHECK-LABEL: define ptr @pr154116_nuw(
; CHECK-SAME: ptr [[P:%.*]], i128 [[I:%.*]]) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: [[TMP1:%.*]] = trunc nuw i128 [[I]] to i64
; CHECK-NEXT: [[TMP1:%.*]] = trunc i128 [[I]] to i64
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr i32, ptr [[P]], i64 [[TMP1]]
; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr i8, ptr [[TMP2]], i64 80
; CHECK-NEXT: ret ptr [[ARRAYIDX2]]
@ -18,13 +16,11 @@ define ptr @pr154116_nuw(ptr %p, i128 %i) {
ret ptr %arrayidx
}
; FIXME: (add (trunc nsw A), (trunc nsw B)) is more poisonous than
; (trunc nsw (add A, B))), so we should for example drop the
; nsw on the trunc when doing the rewrite here.
; Verify that we drop "nsw" from trunc.
define ptr @pr154116_nsw(ptr %p, i128 %i) {
; CHECK-LABEL: define ptr @pr154116_nsw(
; CHECK-SAME: ptr [[P:%.*]], i128 [[I:%.*]]) #[[ATTR0]] {
; CHECK-NEXT: [[TMP1:%.*]] = trunc nsw i128 [[I]] to i64
; CHECK-NEXT: [[TMP1:%.*]] = trunc i128 [[I]] to i64
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr i32, ptr [[P]], i64 [[TMP1]]
; CHECK-NEXT: [[ARRAYIDX2:%.*]] = getelementptr i8, ptr [[TMP2]], i64 4
; CHECK-NEXT: ret ptr [[ARRAYIDX2]]