[llvm] Use ABI instead of preferred alignment for const prop checks

[PiJoules]

We'd hit an assertion checking proper alignment for an i8 when building chromium because we used the prefered alignment (which is 4 bytes) instead of the ABI alignment (which is 1 byte). The ABI alignment should be used because that's the actual alignment needed to load a constant from the vtable.

This also updates the two virtual-const-prop-small-alignment-* to explicitly give ABI alignments for i64s.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (PiJoules)

Changes

We'd hit an assertion checking proper alignment for an i8 when building chromium because we used the prefered alignment (which is 4 bytes) instead of the ABI alignment (which is 1 byte). The ABI alignment should be used because that's the actual alignment needed to load a constant from the vtable.

This also updates the two virtual-const-prop-small-alignment-* to explicitly give ABI alignments for i64s.

---

Full diff: https://github.com/llvm/llvm-project/pull/142500.diff

3 Files Affected:

• (modified) llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp (+1-1)
• (modified) llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-32.ll (+1-1)
• (modified) llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-64.ll (+1-1)

diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 3a25255d0a4c8..a7d9f3ba24b24 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1845,7 +1845,7 @@ bool DevirtModule::tryVirtualConstProp(
   if (BitWidth > 64)
     return false;
 
-  Align TypeAlignment = M.getDataLayout().getPrefTypeAlign(RetType);
+  Align TypeAlignment = M.getDataLayout().getABIIntegerTypeAlignment(BitWidth);
 
   // Make sure that each function is defined, does not access memory, takes at
   // least one argument, does not use its first argument (which we assume is
diff --git a/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-32.ll b/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-32.ll
index ab76f2c22e343..d501694bc4737 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-32.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-32.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility %s | FileCheck %s
 
 ;; This target uses 32-bit sized and aligned pointers.
-target datalayout = "e-p:32:32"
+target datalayout = "e-p:32:32-i64:64"
 
 ;; Constant propagation should be agnostic towards sections.
 ;; Also the new global should be in the original vtable's section.
diff --git a/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-64.ll b/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-64.ll
index c83fbc6ed5a19..b8e4a25bdc88d 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-64.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-small-alignment-64.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility %s | FileCheck %s
 
 ;; This target uses 64-bit sized and aligned pointers.
-target datalayout = "e-p:64:64"
+target datalayout = "e-p:64:64-i64:64"
 
 ;; Constant propagation should be agnostic towards sections.
 ;; Also the new global should be in the original vtable's section.

[ilovepi]

Why is the only change in the test to the data layout string? Shouldn't there be some check that the case you've brought up is handled?

[PiJoules]

Why is the only change in the test to the data layout string? Shouldn't there be some check that the case you've brought up is handled?

I can add another test that asserts the ABI alignment is used instead of the preferred alignment. Existing tests only specified the preferred alignment so the ABI alignment could be anything which led to different codegen (for example, some i8 constants weren't propagated since they had an ABI alignment of 4). I updated the tests just to be explicit about the ABI alignment since we were previously explicit about the preferred alignment.

[ilovepi]

I can add another test that asserts the ABI alignment is used instead of the preferred alignment. Existing tests only specified the preferred alignment so the ABI alignment could be anything which led to different codegen (for example, some i8 constants weren't propagated since they had an ABI alignment of 4). I updated the tests just to be explicit about the ABI alignment since we were previously explicit about the preferred alignment.

I think that's preferable.

[PiJoules]

I can add another test that asserts the ABI alignment is used instead of the preferred alignment. Existing tests only specified the preferred alignment so the ABI alignment could be anything which led to different codegen (for example, some i8 constants weren't propagated since they had an ABI alignment of 4). I updated the tests just to be explicit about the ABI alignment since we were previously explicit about the preferred alignment.

I think that's preferable.

Done

[ilovepi]

LGTM.