From ca99994b868a0ac859857b7b42e16c15a30aa28c Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Thu, 23 Oct 2025 09:15:41 -0400 Subject: [PATCH] [SDAG] Preserve InBounds in DAGCombines This PR preserves the InBounds flag (#162477) where possible in PTRADD-related DAGCombines. We can't preserve them in all the cases that we could in the analogous GISel change (#152495) because SDAG usually represents pointers as integers, which means that pointer provenance is not preserved between PTRADD operations (see the discussion at PR #162477 for more details). This PR marks the places in the DAGCombiner where this is relevant explicitly. For SWDEV-516125. --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index cf221bba1e3a3..6aa8a4a4a202a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -2715,6 +2715,12 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) { (N->getFlags() & N0->getFlags()) & SDNodeFlags::NoUnsignedWrap; SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}, Flags); AddToWorklist(Add.getNode()); + // We can't set InBounds even if both original ptradds were InBounds and + // NUW: SDAG usually represents pointers as integers, therefore, the + // matched pattern behaves as if it had implicit casts: + // (ptradd inbounds (inttoptr (ptrtoint (ptradd inbounds x, y))), z) + // The outer inbounds ptradd might therefore rely on a provenance that x + // does not have. return DAG.getMemBasePlusOffset(X, Add, DL, Flags); } } @@ -2740,6 +2746,12 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) { // that. SDNodeFlags Flags = (N->getFlags() & N0->getFlags()) & SDNodeFlags::NoUnsignedWrap; + // We can't set InBounds even if both original ptradds were InBounds and + // NUW: SDAG usually represents pointers as integers, therefore, the + // matched pattern behaves as if it had implicit casts: + // (ptradd inbounds (inttoptr (ptrtoint (ptradd inbounds GA, v))), c) + // The outer inbounds ptradd might therefore rely on a provenance that + // GA does not have. SDValue Inner = DAG.getMemBasePlusOffset(GAValue, N1, DL, Flags); AddToWorklist(Inner.getNode()); return DAG.getMemBasePlusOffset(Inner, N0.getOperand(1), DL, Flags); @@ -2763,8 +2775,13 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) { bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z); // If both additions in the original were NUW, reassociation preserves that. - SDNodeFlags ReassocFlags = - (N->getFlags() & N1->getFlags()) & SDNodeFlags::NoUnsignedWrap; + SDNodeFlags CommonFlags = N->getFlags() & N1->getFlags(); + SDNodeFlags ReassocFlags = CommonFlags & SDNodeFlags::NoUnsignedWrap; + if (CommonFlags.hasNoUnsignedWrap()) { + // If both operations are NUW and the PTRADD is inbounds, the offests are + // both non-negative, so the reassociated PTRADDs are also inbounds. + ReassocFlags |= N->getFlags() & SDNodeFlags::InBounds; + } if (ZIsConstant != YIsConstant) { if (YIsConstant)