Skip to content

[ConstantHoisting] Don't attempt to hoist ConstantInt vectors. #85416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

paulwalker-arm
Copy link
Collaborator

The pass uses the TTI hook getIntImmCostIntrin that only supports scalar integer types. Whilst hoisting expensive vector constant is likely worthwhile, this is new behaviour and so I've followed the path taken by the GEP variant of collectConstantCandidates and simply bail for vector types.

@llvmbot
Copy link
Member

llvmbot commented Mar 15, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Paul Walker (paulwalker-arm)

Changes

The pass uses the TTI hook getIntImmCostIntrin that only supports scalar integer types. Whilst hoisting expensive vector constant is likely worthwhile, this is new behaviour and so I've followed the path taken by the GEP variant of collectConstantCandidates and simply bail for vector types.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstantHoisting.cpp (+3)
  • (modified) llvm/test/Transforms/ConstantHoisting/AArch64/large-immediate.ll (+31-6)
diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index 49f8761a139232..c12dc281b0e3c4 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -363,6 +363,9 @@ SetVector<Instruction *> ConstantHoistingPass::findConstantInsertionPoint(
 void ConstantHoistingPass::collectConstantCandidates(
     ConstCandMapType &ConstCandMap, Instruction *Inst, unsigned Idx,
     ConstantInt *ConstInt) {
+  if (ConstInt->getType()->isVectorTy())
+    return;
+
   InstructionCost Cost;
   // Ask the target about the cost of materializing the constant for the given
   // instruction and operand index.
diff --git a/llvm/test/Transforms/ConstantHoisting/AArch64/large-immediate.ll b/llvm/test/Transforms/ConstantHoisting/AArch64/large-immediate.ll
index 196a104adc0233..6d8890d71e2be9 100644
--- a/llvm/test/Transforms/ConstantHoisting/AArch64/large-immediate.ll
+++ b/llvm/test/Transforms/ConstantHoisting/AArch64/large-immediate.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
-; RUN: opt -mtriple=arm64-darwin-unknown -S -passes=consthoist < %s | FileCheck %s
+; RUN: opt -mtriple=arm64-darwin-unknown -S -passes=consthoist < %s | FileCheck %s --check-prefixes=CHECK,CV
+; RUN: opt -mtriple=arm64-darwin-unknown -S -passes=consthoist -use-constant-int-for-fixed-length-splat -use-constant-int-for-scalable-splat < %s | FileCheck %s --check-prefixes=CHECK,CI
 
 define i128 @test1(i128 %a) {
 ; CHECK-LABEL: define i128 @test1(
@@ -122,13 +123,37 @@ define i64 @sdiv_minsize(i64 %a) minsize {
 }
 
 define <2 x i64> @sdiv_v2i64(<2 x i64> %a) {
-; CHECK-LABEL: define <2 x i64> @sdiv_v2i64(
-; CHECK-SAME: <2 x i64> [[A:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = sdiv <2 x i64> [[A]], <i64 4294967087, i64 4294967087>
-; CHECK-NEXT:    [[TMP2:%.*]] = add <2 x i64> [[TMP1]], <i64 4294967087, i64 4294967087>
-; CHECK-NEXT:    ret <2 x i64> [[TMP2]]
+; CV-LABEL: define <2 x i64> @sdiv_v2i64(
+; CV-SAME: <2 x i64> [[A:%.*]]) {
+; CV-NEXT:    [[TMP1:%.*]] = sdiv <2 x i64> [[A]], <i64 4294967087, i64 4294967087>
+; CV-NEXT:    [[TMP2:%.*]] = add <2 x i64> [[TMP1]], <i64 4294967087, i64 4294967087>
+; CV-NEXT:    ret <2 x i64> [[TMP2]]
+;
+; CI-LABEL: define <2 x i64> @sdiv_v2i64(
+; CI-SAME: <2 x i64> [[A:%.*]]) {
+; CI-NEXT:    [[TMP1:%.*]] = sdiv <2 x i64> [[A]], splat (i64 4294967087)
+; CI-NEXT:    [[TMP2:%.*]] = add <2 x i64> [[TMP1]], splat (i64 4294967087)
+; CI-NEXT:    ret <2 x i64> [[TMP2]]
 ;
   %1 = sdiv <2 x i64> %a, <i64 4294967087, i64 4294967087>
   %2 = add <2 x i64> %1, <i64 4294967087, i64 4294967087>
   ret <2 x i64> %2
 }
+
+define <vscale x 2 x i64> @sdiv_nxv2i64(<vscale x 2 x i64> %a) {
+; CV-LABEL: define <vscale x 2 x i64> @sdiv_nxv2i64(
+; CV-SAME: <vscale x 2 x i64> [[A:%.*]]) {
+; CV-NEXT:    [[TMP1:%.*]] = sdiv <vscale x 2 x i64> [[A]], shufflevector (<vscale x 2 x i64> insertelement (<vscale x 2 x i64> poison, i64 4294967087, i64 0), <vscale x 2 x i64> poison, <vscale x 2 x i32> zeroinitializer)
+; CV-NEXT:    [[TMP2:%.*]] = add <vscale x 2 x i64> [[TMP1]], shufflevector (<vscale x 2 x i64> insertelement (<vscale x 2 x i64> poison, i64 4294967087, i64 0), <vscale x 2 x i64> poison, <vscale x 2 x i32> zeroinitializer)
+; CV-NEXT:    ret <vscale x 2 x i64> [[TMP2]]
+;
+; CI-LABEL: define <vscale x 2 x i64> @sdiv_nxv2i64(
+; CI-SAME: <vscale x 2 x i64> [[A:%.*]]) {
+; CI-NEXT:    [[TMP1:%.*]] = sdiv <vscale x 2 x i64> [[A]], splat (i64 4294967087)
+; CI-NEXT:    [[TMP2:%.*]] = add <vscale x 2 x i64> [[TMP1]], splat (i64 4294967087)
+; CI-NEXT:    ret <vscale x 2 x i64> [[TMP2]]
+;
+  %1 = sdiv <vscale x 2 x i64> %a, splat (i64 4294967087)
+  %2 = add <vscale x 2 x i64> %1, splat (i64 4294967087)
+  ret <vscale x 2 x i64> %2
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine for now...

The pass uses the TTI hook getIntImmCostIntrin that only supports
scalar integer types. Whilst hoisting expensive vector constant is
likely worthwhile, this is new behaviour and so I've followed the
path taken by the GEP variant of collectConstantCandidates and
simply bail for vector types.
@paulwalker-arm paulwalker-arm force-pushed the vector-constant-hositing-fix branch from 6be9b75 to 0fb9141 Compare March 15, 2024 16:16
@paulwalker-arm paulwalker-arm merged commit a60deaa into llvm:main Mar 18, 2024
@paulwalker-arm paulwalker-arm deleted the vector-constant-hositing-fix branch March 19, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants