LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 44877 - [CodeGenPrepare] Store splitting creates invalid alignments on big-endian targets, and under-aligned stores on all targets
Summary: [CodeGenPrepare] Store splitting creates invalid alignments on big-endian tar...
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Common Code Generator Code (show other bugs)
Version: trunk
Hardware: PC Linux
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks: release-10.0.0
  Show dependency tree
 
Reported: 2020-02-11 23:54 PST by Clement Courbet
Modified: 2020-02-19 09:46 PST (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments
cherry-pick of rG15488ff24b4a on top of 10 (9.74 KB, patch)
2020-02-12 05:34 PST, Clement Courbet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Clement Courbet 2020-02-11 23:54:37 PST

    
Comment 1 Clement Courbet 2020-02-12 00:42:58 PST
Full discussion: https://reviews.llvm.org/D74311
Comment 2 Clement Courbet 2020-02-12 01:58:29 PST
Fixed by rG15488ff24b4a
Comment 3 Hans Wennborg 2020-02-12 04:44:28 PST
From the code review:
"We probably want to backport the big-endian fix into 10.0 in some form?"

Clement, can you help with this? I tried cherry-picking the whole commit over but it doesn't apply cleanly.
Comment 4 Clement Courbet 2020-02-12 04:56:06 PST
> Clement, can you help with this? I tried cherry-picking the whole commit
> over but it doesn't apply cleanly.

I guess the migration to typed Align() had not made it to 10.0.

I'll need to do a manual merge. How can I get the patch to you afterwards (sorry, first time this happens to me) ? Thanks.
Comment 5 Clement Courbet 2020-02-12 05:34:32 PST
Created attachment 23122 [details]
cherry-pick of rG15488ff24b4a on top of 10
Comment 6 Hans Wennborg 2020-02-12 06:31:49 PST
(In reply to Clement Courbet from comment #5)
> Created attachment 23122 [details]
> cherry-pick of rG15488ff24b4a on top of 10

Thanks! Pushed that as b3cf70427eb1e97d9b89ba6e9298c280c8a32c74
Comment 7 Shoaib Meenai 2020-02-18 16:40:06 PST
I’m seeing a failure related to the relevant commit on the release/10.x branch (b3cf70427eb1e97d9b89ba6e9298c280c8a32c74). Specifically, I see a failure in the test “Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll”. Here are the results from me locally running this:

apl@apl-mbp ~/code/llvm-project/build (release/10.x) $ ./bin/llvm-lit -sv test/Transforms/CodeGenPrepare/PowerPC
FAIL: LLVM :: Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll (1 of 1)
******************** TEST 'LLVM :: Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll' FAILED ********************
Script:
--
: 'RUN: at line 2';   /Users/apl/code/llvm-project/build/bin/opt -S -codegenprepare -mtriple=powerpc64-unknown-linux-gnu -data-layout="E-m:e-i64:64-n32:64" -force-split-store < /Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll  | /Users/apl/code/llvm-project/build/bin/FileCheck --check-prefixes=ALL,BE /Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll
: 'RUN: at line 3';   /Users/apl/code/llvm-project/build/bin/opt -S -codegenprepare -mtriple=powerpc64le-unknown-linux-gnu -data-layout="e-m:e-i64:64-n32:64" -force-split-store < /Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll  | /Users/apl/code/llvm-project/build/bin/FileCheck --check-prefixes=ALL,LE /Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll
--
Exit Code: 1

Command Output (stderr):
--
/Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll:12:12: error: BE-NEXT: expected string not found in input
; BE-NEXT: [[TMP1:%.*]] = bitcast i64* [[P:%.*]] to i32*
           ^
<stdin>:12:2: note: scanning from here
 store i64 %o, i64* %p, align 1
 ^
/Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll:48:12: error: BE-NEXT: expected string not found in input
; BE-NEXT: [[TMP1:%.*]] = bitcast i64* [[P:%.*]] to i32*
           ^
<stdin>:22:2: note: scanning from here
 store i64 %o, i64* %p, align 2
 ^
/Users/apl/code/llvm-project/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll:84:12: error: BE-NEXT: expected string not found in input
; BE-NEXT: [[TMP1:%.*]] = bitcast i64* [[P:%.*]] to i32*
           ^
<stdin>:32:2: note: scanning from here
 store i64 %o, i64* %p, align 8
 ^

--

********************


It looks like the test is expecting to see several things between the or and the store but running the first invocation of opt yields this:
define void @split_store_align1(float %x, i64* %p) {
  %b = bitcast float %x to i32
  %z = zext i32 0 to i64
  %s = shl nuw nsw i64 %z, 32
  %z2 = zext i32 %b to i64
  %o = or i64 %s, %z2
  store i64 %o, i64* %p, align 1
  ret void
}
Comment 8 Shoaib Meenai 2020-02-18 16:40:38 PST
(posting that on behalf of a coworker who doesn't have a Bugzilla account yet, but it's blocking our internal integration of 10.0)
Comment 9 Hans Wennborg 2020-02-19 04:11:17 PST
That's strange. The test passes on my machine, and the output of the first run line looks like this:

$ /work/llvm.monorepo.rel/build.release/bin/opt -S -codegenprepare -mtriple=powerpc64-unknown-linux-gnu -data-layout="E-m:e-i64:64-n32:64" -force-split-store < /work/llvm.monorepo.rel/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll | head -19
; ModuleID = '<stdin>'
source_filename = "<stdin>"
target datalayout = "E-m:e-i64:64-n32:64"
target triple = "powerpc64-unknown-linux-gnu"

define void @split_store_align1(float %x, i64* %p) {
  %b = bitcast float %x to i32
  %z = zext i32 0 to i64
  %s = shl nuw nsw i64 %z, 32
  %z2 = zext i32 %b to i64
  %o = or i64 %s, %z2
  %1 = bitcast i64* %p to i32*
  %2 = getelementptr i32, i32* %1, i32 1
  store i32 %b, i32* %2, align 1
  %3 = bitcast i64* %p to i32*
  store i32 0, i32* %3, align 1
  ret void
}


Could it be due to a difference in host architecture or build configuration or something?
Comment 10 Clement Courbet 2020-02-19 04:27:07 PST
Hm. I think I know what's happening. Are you by any chance compiling without support for PowerPC ? I just noticed that I failed to backport the base commit for the change, which included the addition of the local lit config checking for PowerPC targets.
Comment 11 Clement Courbet 2020-02-19 04:37:57 PST
Indeed, I could reproduce by compiling only with X86 support.
I have committed be45a5a4092d678c992ac5d32e4b41da9367989a, which fixes the issue on my machine.
Comment 12 Hans Wennborg 2020-02-19 04:42:16 PST
(In reply to Clement Courbet from comment #11)
> Indeed, I could reproduce by compiling only with X86 support.
> I have committed be45a5a4092d678c992ac5d32e4b41da9367989a, which fixes the
> issue on my machine.

Thanks!
Comment 13 Shoaib Meenai 2020-02-19 09:46:59 PST
Yup, we weren't enabling the PowerPC target. Thanks for the fix!