[clang] [clang] Emit nuw GEPs for array subscript expressions (PR #103088)
https://github.com/hazzlim closed https://github.com/llvm/llvm-project/pull/103088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Emit nuw GEPs for array subscript expressions (PR #103088)
hazzlim wrote: > Can this be closed in favor of #105496? Yes good point - will close this and open a PR to Reland #105496. https://github.com/llvm/llvm-project/pull/103088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[clang] Add nuw attribute to GEPs (#105496)" (PR #107257)
hazzlim wrote: Nice - CI seems happy, so I plan to land this later today if no objections. https://github.com/llvm/llvm-project/pull/107257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland "[clang] Add nuw attribute to GEPs (#105496)" (PR #107257)
https://github.com/hazzlim closed https://github.com/llvm/llvm-project/pull/107257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [polly] [clang] Generate nuw GEPs for struct member accesses (PR #99538)
https://github.com/hazzlim closed https://github.com/llvm/llvm-project/pull/99538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Emit nuw GEPs for array subscript expressions (PR #103088)
https://github.com/hazzlim created
https://github.com/llvm/llvm-project/pull/103088
Generate nuw GEPs for array subscript expressions where the base address points
to the base of a constant size array and the index is unsigned.
>From 1eca1fe3d73c9832bde3e09a93f8d0a2a2bb3698 Mon Sep 17 00:00:00 2001
From: Hari Limaye
Date: Thu, 13 Jun 2024 21:06:50 +
Subject: [PATCH] [clang] Emit nuw GEPs for array subscript expressions
Generate nuw GEPs for array subscript expressions where the base address
points to the base of a constant size array and the index is unsigned.
---
clang/lib/CodeGen/CGBuilder.h | 6 ++-
clang/lib/CodeGen/CGExpr.cpp | 41 ---
clang/lib/CodeGen/CGExprScalar.cpp| 38 ++---
clang/lib/CodeGen/CodeGenFunction.h | 9 ++--
clang/test/CodeGen/PowerPC/ppc-emmintrin.c| 12 +++---
clang/test/CodeGen/PowerPC/ppc-xmmintrin.c| 16
.../CodeGenCXX/pr45964-decomp-transform.cpp | 2 +-
7 files changed, 75 insertions(+), 49 deletions(-)
diff --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index 08730a6a6672a1..b8036cf6e6a306 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -14,6 +14,7 @@
#include "CodeGenTypeCache.h"
#include "llvm/Analysis/Utils/Local.h"
#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/GEPNoWrapFlags.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Type.h"
@@ -334,9 +335,10 @@ class CGBuilderTy : public CGBuilderBaseTy {
Address CreateGEP(Address Addr, ArrayRef IdxList,
llvm::Type *ElementType, CharUnits Align,
-const Twine &Name = "") {
+const Twine &Name = "",
+llvm::GEPNoWrapFlags NW = llvm::GEPNoWrapFlags::none()) {
llvm::Value *Ptr = emitRawPointerFromAddress(Addr);
-return RawAddress(CreateGEP(Addr.getElementType(), Ptr, IdxList, Name),
+return RawAddress(CreateGEP(Addr.getElementType(), Ptr, IdxList, Name, NW),
ElementType, Align);
}
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index f93f8dda0bd29a..74fa7594baeb9e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3959,13 +3959,13 @@ static llvm::Value
*emitArraySubscriptGEP(CodeGenFunction &CGF,
static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
ArrayRef indices,
llvm::Type *elementType, bool inbounds,
- bool signedIndices, SourceLocation loc,
- CharUnits align,
+ bool nuw, bool signedIndices,
+ SourceLocation loc, CharUnits align,
const llvm::Twine &name = "arrayidx") {
if (inbounds) {
return CGF.EmitCheckedInBoundsGEP(addr, indices, elementType,
signedIndices,
CodeGenFunction::NotSubtraction, loc,
- align, name);
+ align, name, nuw);
} else {
return CGF.Builder.CreateGEP(addr, indices, elementType, align, name);
}
@@ -4060,7 +4060,7 @@ static bool IsPreserveAIArrayBase(CodeGenFunction &CGF,
const Expr *ArrayBase) {
static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
ArrayRef indices,
- QualType eltType, bool inbounds,
+ QualType eltType, bool inbounds, bool nuw,
bool signedIndices, SourceLocation loc,
QualType *arrayType = nullptr,
const Expr *Base = nullptr,
@@ -4091,7 +4091,7 @@ static Address emitArraySubscriptGEP(CodeGenFunction
&CGF, Address addr,
if (!LastIndex ||
(!CGF.IsInPreservedAIRegion && !IsPreserveAIArrayBase(CGF, Base))) {
addr = emitArraySubscriptGEP(CGF, addr, indices,
- CGF.ConvertTypeForMem(eltType), inbounds,
+ CGF.ConvertTypeForMem(eltType), inbounds, nuw,
signedIndices, loc, eltAlign, name);
return addr;
} else {
@@ -4214,7 +4214,7 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const
ArraySubscriptExpr *E,
QualType EltType = LV.getType()->castAs()->getElementType();
Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true,
- SignedIndices, E->getExprLoc());
+ /*nuw=*/false, SignedIndices,
E->getExprLoc());
return MakeAddrLValue(Addr, EltType, LV.getBaseInfo(),
CGM.getTBAAInfoForSubobject(LV, EltType));
}
@@ -4245,7 +4245,7 @@ LValue CodeGenFunctio
[clang] [clang] Emit nuw GEPs for array subscript expressions (PR #103088)
hazzlim wrote: This seems correct from my reading of the relevant parts of the C/C++ standard, regarding pointer arithmetic and array subscript expressions. It does actually seem like the restriction to unsigned indices here is actually unnecessary, and adding `nuw` is also valid for signed indices here - but I wanted to check if people agree on that. https://github.com/llvm/llvm-project/pull/103088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Generate nuw GEPs for struct member accesses (PR #99538)
hazzlim wrote: > `CGBuilder` is just aiming to provide an `Address` overload of LLVM's > `CreateStructGEP`. It seems wrong for two overloads to have subtly different > behavior. > > Is there a reason why LLVM's `CreateStructGEP` doesn't add `nuw` itself? Good point - I can see that it makes more sense to add the flag there. I can rework this patch to do this. Before doing so - do people think it still makes sense to do add the flag in ‘llvm::CreateStructGEP’, given that as @nikic points out we can infer the flag at the IR level? https://github.com/llvm/llvm-project/pull/99538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Generate nuw GEPs for struct member accesses (PR #99538)
hazzlim wrote: > > `CGBuilder` is just aiming to provide an `Address` overload of LLVM's > > `CreateStructGEP`. It seems wrong for two overloads to have subtly > > different behavior. > > Is there a reason why LLVM's `CreateStructGEP` doesn't add `nuw` itself? > > Good point - I can see that it makes more sense to add the flag there. > > I can rework this patch to do this. Before doing so - do people think it > still makes sense to do add the flag in ‘llvm::CreateStructGEP’, given that > as @nikic points out we can infer the flag at the IR level? @rjmccall I've reworked this patch so that the nuw flag is added in LLVM's `CreateStructGEP` directly. https://github.com/llvm/llvm-project/pull/99538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Generate nuw GEPs for struct member accesses (PR #99538)
hazzlim wrote: This seemed like the right place to make the change - please let me know if not! I've not restricted this to a subset of languages because it seems fairly unambiguous that nuw holds for these inbounds struct member accesses, but it could be limited to e.g. C/C++ if that is more appropriate. https://github.com/llvm/llvm-project/pull/99538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add nuw attribute to GEPs (PR #105496)
hazzlim wrote: I plan to land this patch later today, unless there are any objections / further comments. Thanks for reviewing @nikic @efriedma-quic https://github.com/llvm/llvm-project/pull/105496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add nuw attribute to GEPs (PR #105496)
https://github.com/hazzlim closed https://github.com/llvm/llvm-project/pull/105496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Add nuw attribute to GEPs" (PR #106343)
hazzlim wrote: > Please let me know if you have a fix for that. I'll land revert in the > morning by PDT. Thank you for flagging this up and opening this @vitalybuka - I'm taking a look now to see if there's an easy enough fix to save reverting. https://github.com/llvm/llvm-project/pull/106343 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add nuw attribute to GEPs (PR #105496)
hazzlim wrote: > @mikaelholmen Thanks for the reproducer, this makes the issue clear. BasicAA > is incorrectly returning NoAlias for the pointers due to #98608. > > The issue is that the `add` gets decomposed, but we preserve the nuw flag, > effectively making it an `add nuw i16 n, -1`. Thanks for identifying this - I'm looking into a fix. Would you prefer I revert #98608 in the meantime @nikic ? https://github.com/llvm/llvm-project/pull/105496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] Generate nuw GEPs for struct member accesses (PR #99538)
@@ -1902,16 +1902,18 @@ class IRBuilderBase {
}
Value *CreateConstGEP2_32(Type *Ty, Value *Ptr, unsigned Idx0, unsigned Idx1,
-const Twine &Name = "") {
+const Twine &Name = "",
+GEPNoWrapFlags NWFlags = GEPNoWrapFlags::none()) {
Value *Idxs[] = {
ConstantInt::get(Type::getInt32Ty(Context), Idx0),
ConstantInt::get(Type::getInt32Ty(Context), Idx1)
};
-if (auto *V = Folder.FoldGEP(Ty, Ptr, Idxs, GEPNoWrapFlags::none()))
+if (auto *V =
+Folder.FoldGEP(Ty, Ptr, Idxs, /*IsInBounds=*/NWFlags.isInBounds()))
hazzlim wrote:
Yes, sorry about that - updated to pass the full `NWFlags` correctly (+
associated test updates).
https://github.com/llvm/llvm-project/pull/99538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add nuw attribute to GEPs (PR #105496)
@@ -7,13 +7,6 @@
struct X { int a[2]; };
extern int bar();
-//.
-// CHECK: @test.i23 = internal global i32 4, align 4
-// CHECK: @i = global i32 4, align 4
-// CHECK: @Arr = global [100 x i32] zeroinitializer, align 16
-// CHECK: @foo2.X = internal global ptr getelementptr (i8, ptr @Arr, i64 196),
align 8
-// CHECK: @foo2.i23 = internal global i32 0, align 4
hazzlim wrote:
Yeah, possibly something that could be addressed in the script if desired - not
sure how much it comes up.
Ok great, I've updated the `--global-value-regex` argument and fixed up the
missing global CHECKs.
https://github.com/llvm/llvm-project/pull/105496
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Emit nuw GEPs for array subscript expressions (PR #103088)
hazzlim wrote: > > adding nuw is also valid for signed indices here > > I don't understand how you think this would work; a-1 and a+-1 are required > to produce the same result. The thinking here RE signed indices was that in this special case, where the base address of the GEP is the start of an array, the index cannot be negative without violating the `inbounds` attribute (alternatively the requirement of a pointer arithmetic expression to evaluate to an element of the array or one past the end, in C/C++ standard terms). Therefore we could prove `nuw` via `inbounds` + `nneg`. I wonder if others agree that this is valid or can see an issue with this? https://github.com/llvm/llvm-project/pull/103088 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
