[llvm] r208640 - [DAGCombiner] Split up an indexed load if only the base pointer value is live
Adam Nemet
anemet at apple.com
Thu Jun 12 16:11:19 PDT 2014
Hal,
Did you guys make any progress on this? I’d like to see this committed back possibly with some fixes.
Let me know if I can help. Is there a PR tracking the issue?
Thanks,
Adam
On May 28, 2014, at 9:57 AM, Adam Nemet <anemet at apple.com> wrote:
> Hi Hal,
>
> Sure, sorry about that. Let me know if I can help in any way.
>
> You may want to try disable the transformation in the presence of TargetConstants. This is what cause the issue in PR19796. Something like:
>
> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index 2d2fd53..ed3c581 100644
> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -7899,7 +7899,8 @@ SDValue DAGCombiner::visitLOAD(SDNode *N) {
> if (!N->hasAnyUseOfValue(0)) {
> SDValue Undef = DAG.getUNDEF(N->getValueType(0));
> SDValue Index;
> - if (N->hasAnyUseOfValue(1)) {
> + if (N->hasAnyUseOfValue(1) &&
> + N->getOperand(1)->getOpcode() != ISD::TargetConstant) {
> Index = SplitIndexingFromLoad(LD);
> // Try to fold the base pointer arithmetic into subsequent loads and
> // stores.
>
> Adam
>
> On May 28, 2014, at 8:45 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>> Adam,
>>
>> Unfortunately, this caused a massive self-hosting failure on ppc64/Linux. I've reverted it (in r209747) while we investigate. Given that, with this change, nearly all regression tests segfault, I hope it will be relatively easy to diagnose.
>>
>> -Hal
>>
>> ----- Original Message -----
>>> From: "Adam Nemet" <anemet at apple.com>
>>> To: llvm-commits at cs.uiuc.edu
>>> Sent: Monday, May 12, 2014 6:00:03 PM
>>> Subject: [llvm] r208640 - [DAGCombiner] Split up an indexed load if only the base pointer value is live
>>>
>>> Author: anemet
>>> Date: Mon May 12 18:00:03 2014
>>> New Revision: 208640
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=208640&view=rev
>>> Log:
>>> [DAGCombiner] Split up an indexed load if only the base pointer value
>>> is live
>>>
>>> Right now the load may not get DCE'd because of the side-effect of
>>> updating
>>> the base pointer.
>>>
>>> This can happen if we lower a read-modify-write of an illegal larger
>>> type
>>> (e.g. i48) such that the modification only affects one of the
>>> subparts (the
>>> lower i32 part but not the higher i16 part). See the testcase.
>>>
>>> In order to spot the dead load we need to revisit it when
>>> SimplifyDemandedBits
>>> decided that the value of the load is masked off. This is the
>>> CommitTargetLoweringOpt piece.
>>>
>>> I checked compile time with ARM64 by sending SPEC bitcode files
>>> through llc.
>>> No measurable change.
>>>
>>> Fixes <rdar://problem/16031651>
>>>
>>> Added:
>>> llvm/trunk/test/CodeGen/ARM64/dagcombiner-dead-indexed-load.ll
>>> Modified:
>>> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=208640&r1=208639&r2=208640&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon May 12
>>> 18:00:03 2014
>>> @@ -167,6 +167,7 @@ namespace {
>>>
>>> bool CombineToPreIndexedLoadStore(SDNode *N);
>>> bool CombineToPostIndexedLoadStore(SDNode *N);
>>> + SDValue SplitIndexingFromLoad(LoadSDNode *LD);
>>> bool SliceUpLoad(SDNode *N);
>>>
>>> void ReplaceLoadWithPromotedLoad(SDNode *Load, SDNode *ExtLoad);
>>> @@ -761,10 +762,14 @@ CommitTargetLoweringOpt(const TargetLowe
>>>
>>> // If the operands of this node are only used by the node, they
>>> will now
>>> // be dead. Make sure to visit them first to delete dead nodes
>>> early.
>>> - for (unsigned i = 0, e = TLO.Old.getNode()->getNumOperands(); i
>>> != e; ++i)
>>> - if (TLO.Old.getNode()->getOperand(i).getNode()->hasOneUse())
>>> - AddToWorkList(TLO.Old.getNode()->getOperand(i).getNode());
>>> -
>>> + for (unsigned i = 0, e = TLO.Old.getNode()->getNumOperands(); i
>>> != e; ++i) {
>>> + SDNode *Op = TLO.Old.getNode()->getOperand(i).getNode();
>>> + // For an operand generating multiple values, one of the
>>> values may
>>> + // become dead allowing further simplification (e.g. split
>>> index
>>> + // arithmetic from an indexed load).
>>> + if (Op->hasOneUse() || Op->getNumValues() > 1)
>>> + AddToWorkList(Op);
>>> + }
>>> DAG.DeleteNode(TLO.Old.getNode());
>>> }
>>> }
>>> @@ -7844,6 +7849,17 @@ bool DAGCombiner::CombineToPostIndexedLo
>>> return false;
>>> }
>>>
>>> +/// \brief Return the base-pointer arithmetic from an indexed \p LD.
>>> +SDValue DAGCombiner::SplitIndexingFromLoad(LoadSDNode *LD) {
>>> + ISD::MemIndexedMode AM = LD->getAddressingMode();
>>> + assert(AM != ISD::UNINDEXED);
>>> + SDValue BP = LD->getOperand(1);
>>> + SDValue Inc = LD->getOperand(2);
>>> + unsigned Opc =
>>> + (AM == ISD::PRE_INC || AM == ISD::POST_INC ? ISD::ADD :
>>> ISD::SUB);
>>> + return DAG.getNode(Opc, SDLoc(LD), BP.getSimpleValueType(), BP,
>>> Inc);
>>> +}
>>> +
>>> SDValue DAGCombiner::visitLOAD(SDNode *N) {
>>> LoadSDNode *LD = cast<LoadSDNode>(N);
>>> SDValue Chain = LD->getChain();
>>> @@ -7880,8 +7896,16 @@ SDValue DAGCombiner::visitLOAD(SDNode *N
>>> } else {
>>> // Indexed loads.
>>> assert(N->getValueType(2) == MVT::Other && "Malformed indexed
>>> loads?");
>>> - if (!N->hasAnyUseOfValue(0) && !N->hasAnyUseOfValue(1)) {
>>> + if (!N->hasAnyUseOfValue(0)) {
>>> SDValue Undef = DAG.getUNDEF(N->getValueType(0));
>>> + SDValue Index;
>>> + if (N->hasAnyUseOfValue(1)) {
>>> + Index = SplitIndexingFromLoad(LD);
>>> + // Try to fold the base pointer arithmetic into subsequent
>>> loads and
>>> + // stores.
>>> + AddUsersToWorkList(N);
>>> + } else
>>> + Index = DAG.getUNDEF(N->getValueType(1));
>>> DEBUG(dbgs() << "\nReplacing.7 ";
>>> N->dump(&DAG);
>>> dbgs() << "\nWith: ";
>>> @@ -7889,8 +7913,7 @@ SDValue DAGCombiner::visitLOAD(SDNode *N
>>> dbgs() << " and 2 other values\n");
>>> WorkListRemover DeadNodes(*this);
>>> DAG.ReplaceAllUsesOfValueWith(SDValue(N, 0), Undef);
>>> - DAG.ReplaceAllUsesOfValueWith(SDValue(N, 1),
>>> -
>>> DAG.getUNDEF(N->getValueType(1)));
>>> + DAG.ReplaceAllUsesOfValueWith(SDValue(N, 1), Index);
>>> DAG.ReplaceAllUsesOfValueWith(SDValue(N, 2), Chain);
>>> removeFromWorkList(N);
>>> DAG.DeleteNode(N);
>>>
>>> Added: llvm/trunk/test/CodeGen/ARM64/dagcombiner-dead-indexed-load.ll
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM64/dagcombiner-dead-indexed-load.ll?rev=208640&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/ARM64/dagcombiner-dead-indexed-load.ll
>>> (added)
>>> +++ llvm/trunk/test/CodeGen/ARM64/dagcombiner-dead-indexed-load.ll
>>> Mon May 12 18:00:03 2014
>>> @@ -0,0 +1,29 @@
>>> +; RUN: llc -mcpu=cyclone < %s | FileCheck %s
>>> +
>>> +target datalayout = "e-i64:64-n32:64-S128"
>>> +target triple = "arm64-apple-ios"
>>> +
>>> +%"struct.SU" = type { i32, %"struct.SU"*, i32*, i32, i32,
>>> %"struct.BO", i32, [5 x i8] }
>>> +%"struct.BO" = type { %"struct.RE" }
>>> +
>>> +%"struct.RE" = type { i32, i32, i32, i32 }
>>> +
>>> +; This is a read-modify-write of some bifields combined into an i48.
>>> It gets
>>> +; legalized into i32 and i16 accesses. Only a single store of zero
>>> to the low
>>> +; i32 part should be live.
>>> +
>>> +; CHECK-LABEL: test:
>>> +; CHECK-NOT: ldr
>>> +; CHECK: str wzr
>>> +; CHECK-NOT: str
>>> +define void @test(%"struct.SU"* nocapture %su) {
>>> +entry:
>>> + %r1 = getelementptr inbounds %"struct.SU"* %su, i64 1, i32 5
>>> + %r2 = bitcast %"struct.BO"* %r1 to i48*
>>> + %r3 = load i48* %r2, align 8
>>> + %r4 = and i48 %r3, -4294967296
>>> + %r5 = or i48 0, %r4
>>> + store i48 %r5, i48* %r2, align 8
>>> +
>>> + ret void
>>> +}
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>
More information about the llvm-commits
mailing list