[llvm-commits] [llvm] r145273 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll test/CodeGen/X86/widen_load-1.ll
Duncan Sands
baldrick at free.fr
Mon Nov 28 12:54:33 PST 2011
Hi Evan,
> DAG combine should not increase alignment of loads / stores with alignment less
> than ABI alignment. These are loads / stores from / to "packed" data structures.
> Their alignments are intentionally under-specified.
this doesn't make any sense to me. Load/store alignment should only be
increased, say from 2 to 4, if it can be proved that the memory being
pointed to is 4 byte aligned. If InferPtrAlignment is working correctly,
it should only be returning 4 if the memory really is 4 byte aligned.
If you have to sometimes ignore what InferPtrAlignment returns because
(presumably) the memory isn't actually 4 byte aligned, doesn't that just
mean that InferPtrAlignment is buggy and should be fixed?
Ciao, Duncan.
>
> rdar://10301431
>
> Added:
> llvm/trunk/test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll
> Modified:
> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> llvm/trunk/test/CodeGen/X86/widen_load-1.ll
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=145273&r1=145272&r2=145273&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Nov 28 14:42:56 2011
> @@ -6206,13 +6206,20 @@
>
> // Try to infer better alignment information than the load already has.
> if (OptLevel != CodeGenOpt::None&& LD->isUnindexed()) {
> - if (unsigned Align = DAG.InferPtrAlignment(Ptr)) {
> - if (Align> LD->getAlignment())
> - return DAG.getExtLoad(LD->getExtensionType(), N->getDebugLoc(),
> - LD->getValueType(0),
> - Chain, Ptr, LD->getPointerInfo(),
> - LD->getMemoryVT(),
> - LD->isVolatile(), LD->isNonTemporal(), Align);
> + unsigned ABIAlign = TLI.getTargetData()->
> + getABITypeAlignment(LD->getMemoryVT().getTypeForEVT(*DAG.getContext()));
> + unsigned LDAlign = LD->getAlignment();
> + // Do not touch loads with explicit alignments that are smaller than ABI
> + // alignment to avoid breaking loads from "packed" types.
> + if (!LDAlign || LDAlign>= ABIAlign) {
> + if (unsigned Align = DAG.InferPtrAlignment(Ptr)) {
> + if (Align> LDAlign)
> + return DAG.getExtLoad(LD->getExtensionType(), N->getDebugLoc(),
> + LD->getValueType(0),
> + Chain, Ptr, LD->getPointerInfo(),
> + LD->getMemoryVT(),
> + LD->isVolatile(), LD->isNonTemporal(), Align);
> + }
> }
> }
>
> @@ -6669,11 +6676,18 @@
>
> // Try to infer better alignment information than the store already has.
> if (OptLevel != CodeGenOpt::None&& ST->isUnindexed()) {
> - if (unsigned Align = DAG.InferPtrAlignment(Ptr)) {
> - if (Align> ST->getAlignment())
> - return DAG.getTruncStore(Chain, N->getDebugLoc(), Value,
> - Ptr, ST->getPointerInfo(), ST->getMemoryVT(),
> - ST->isVolatile(), ST->isNonTemporal(), Align);
> + unsigned ABIAlign = TLI.getTargetData()->
> + getABITypeAlignment(ST->getMemoryVT().getTypeForEVT(*DAG.getContext()));
> + unsigned STAlign = ST->getAlignment();
> + // Do not touch stores with explicit alignments that are smaller than ABI
> + // alignment to avoid breaking stores from "packed" types.
> + if (!STAlign || STAlign>= ABIAlign) {
> + if (unsigned Align = DAG.InferPtrAlignment(Ptr)) {
> + if (Align> STAlign)
> + return DAG.getTruncStore(Chain, N->getDebugLoc(), Value,
> + Ptr, ST->getPointerInfo(), ST->getMemoryVT(),
> + ST->isVolatile(), ST->isNonTemporal(),Align);
> + }
> }
> }
>
>
> Added: llvm/trunk/test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll?rev=145273&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/2011-11-28-DAGCombineBug.ll Mon Nov 28 14:42:56 2011
> @@ -0,0 +1,38 @@
> +; RUN: llc< %s -mtriple=thumbv7-apple-ios5.0.0 | FileCheck %s
> +; rdar://10464621
> +
> +; DAG combine increases loads from packed types. ARM load / store optimizer then
> +; combined them into a ldm which causes runtime exception.
> +
> +%struct.InformationBlock = type<{ i32, %struct.FlagBits, %struct.FlagBits }>
> +%struct.FlagBits = type<{ [4 x i32] }>
> +
> + at infoBlock = external global %struct.InformationBlock
> +
> +define hidden void @foo() {
> +; CHECK: foo:
> +; CHECK: ldr.w
> +; CHECK: ldr.w
> +; CHECK-NOT: ldm
> +entry:
> + %tmp13 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 1, i32 0, i32 0), align 1
> + %tmp15 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 1, i32 0, i32 1), align 1
> + %tmp17 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 1, i32 0, i32 2), align 1
> + %tmp19 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 1, i32 0, i32 3), align 1
> + %tmp = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 2, i32 0, i32 0), align 1
> + %tmp3 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 2, i32 0, i32 1), align 1
> + %tmp4 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 2, i32 0, i32 2), align 1
> + %tmp5 = load i32* getelementptr inbounds (%struct.InformationBlock* @infoBlock, i32 0, i32 2, i32 0, i32 3), align 1
> + %insert21 = insertvalue [4 x i32] undef, i32 %tmp13, 0
> + %insert23 = insertvalue [4 x i32] %insert21, i32 %tmp15, 1
> + %insert25 = insertvalue [4 x i32] %insert23, i32 %tmp17, 2
> + %insert27 = insertvalue [4 x i32] %insert25, i32 %tmp19, 3
> + %insert = insertvalue [4 x i32] undef, i32 %tmp, 0
> + %insert7 = insertvalue [4 x i32] %insert, i32 %tmp3, 1
> + %insert9 = insertvalue [4 x i32] %insert7, i32 %tmp4, 2
> + %insert11 = insertvalue [4 x i32] %insert9, i32 %tmp5, 3
> + tail call void @bar([4 x i32] %insert27, [4 x i32] %insert11)
> + ret void
> +}
> +
> +declare void @bar([4 x i32], [4 x i32])
>
> Modified: llvm/trunk/test/CodeGen/X86/widen_load-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/widen_load-1.ll?rev=145273&r1=145272&r2=145273&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/widen_load-1.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/widen_load-1.ll Mon Nov 28 14:42:56 2011
> @@ -1,5 +1,6 @@
> ; RUN: llc %s -o - -march=x86-64 -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
> ; PR4891
> +; PR5626
>
> ; This load should be before the call, not after.
>
> @@ -20,7 +21,7 @@
> store i32 0, i32* %changed, align 4
> %r2 = getelementptr float* bitcast ([20 x i64]* @compl to float*), i64 32 ;<float*> [#uses=1]
> %r3 = bitcast float* %r2 to<2 x float>* ;<<2 x float>*> [#uses=1]
> - %r4 = load<2 x float>* %r3, align 4 ;<<2 x float>> [#uses=1]
> + %r4 = load<2 x float>* %r3, align 8 ;<<2 x float>> [#uses=1]
> call void @killcommon(i32* %changed)
> br label %"file complex.c, line 34, bb4"
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list