[PATCH] D48029: [DAGCombine] Fix alignment for offset loads/stores
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 11 08:31:31 PDT 2018
dmgreen created this revision.
dmgreen added reviewers: spatel, niravd, efriedma, hfinkel, resistor.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.
This test case (which I hope is free on UB), has two stores of 0 to offsets 20 and 24 in a chunk of memory:
store i32 0, i32* %helper.20.32
store i32 0, i32* %helper.24.32, align 8
A 64 bit load, aligned to 4 bytes:
%load.helper.20.64 = load i64, i64* %helper.20.64, align 4
This is on AArch32, so during type legalisation the i64 load is split into two 32bit loads. The second of them:
t35: i32,ch = load<(load 4 from %ir.helper.20.64 + 4)> t21, t37, undef:i32
gets marked as being align 8 (note: the base+offset is align 8, not the base). This is then deemed to not alias with the load to %helper.24.32 as the alignment just set is taken as the base alignment, not the base+offset alignment.
The test case seems to need a <4 x i32> which on ARM is converted to a VLD1_UPD. I believe this pushes certain optimisation back later after legalisation. Originally it needed -combiner-global-alias-analysis, but this version shows the same error without.
Here I've set the updated alignment only if the alignment hold true for the base.
https://reviews.llvm.org/D48029
Files:
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
test/CodeGen/ARM/alias_align.ll
Index: test/CodeGen/ARM/alias_align.ll
===================================================================
--- /dev/null
+++ test/CodeGen/ARM/alias_align.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv8-arm-none-eabi"
+
+; Check the loads happen after the stores (note: directly returning 0 is also valid)
+; CHECK-LABEL: somesortofhash
+; CHECK-NOT: ldr
+; CHECK: str
+
+define i64 @somesortofhash() {
+entry:
+ %helper = alloca i8, i32 64, align 8
+ %helper.0.4x32 = bitcast i8* %helper to <4 x i32>*
+ %helper.20 = getelementptr inbounds i8, i8* %helper, i32 20
+ %helper.24 = getelementptr inbounds i8, i8* %helper, i32 24
+ store <4 x i32> zeroinitializer, <4 x i32>* %helper.0.4x32, align 8
+ %helper.20.32 = bitcast i8* %helper.20 to i32*
+ %helper.24.32 = bitcast i8* %helper.24 to i32*
+ store i32 0, i32* %helper.20.32
+ store i32 0, i32* %helper.24.32, align 8
+ %helper.20.64 = bitcast i8* %helper.20 to i64*
+ %load.helper.20.64 = load i64, i64* %helper.20.64, align 4
+ ret i64 %load.helper.20.64
+}
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -12182,7 +12182,8 @@
// 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->getMemOperand()->getBaseAlignment()) {
+ if (Align > LD->getOriginalAlignment() &&
+ LD->getSrcValueOffset() % Align == 0) {
SDValue NewLoad = DAG.getExtLoad(
LD->getExtensionType(), SDLoc(N), LD->getValueType(0), Chain, Ptr,
LD->getPointerInfo(), LD->getMemoryVT(), Align,
@@ -14189,7 +14190,8 @@
// 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()) {
+ if (Align > ST->getOriginalAlignment() &&
+ ST->getSrcValueOffset() % Align == 0) {
SDValue NewStore =
DAG.getTruncStore(Chain, SDLoc(N), Value, Ptr, ST->getPointerInfo(),
ST->getMemoryVT(), Align,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48029.150761.patch
Type: text/x-patch
Size: 2417 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180611/f1508772/attachment.bin>
More information about the llvm-commits
mailing list