Skip to content

Conversation

@dcaballe
Copy link
Contributor

@dcaballe dcaballe commented Jun 7, 2025

This PR is part of the last step to remove vector.extractelement and vector.insertelement ops (RFC: https://discourse.llvm.org/t/rfc-psa-remove-vector-extractelement-and-vector-insertelement-ops-in-favor-of-vector-extract-and-vector-insert-ops).
It replaces vector.insertelement and vector.extractelement with vector.insert and vector.extract in Flang. It looks like no lit tests are impacted?

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jun 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 7, 2025

@llvm/pr-subscribers-flang-codegen

@llvm/pr-subscribers-flang-fir-hlfir

Author: Diego Caballero (dcaballe)

Changes

This PR is part of the last step to remove vector.extractelement and vector.insertelement ops (RFC: https://discourse.llvm.org/t/rfc-psa-remove-vector-extractelement-and-vector-insertelement-ops-in-favor-of-vector-extract-and-vector-insert-ops).
It replaces vector.insertelement and vector.extractelement with vector.insert and vector.extract in Flang. It looks like no lit tests are impacted?


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

1 Files Affected:

  • (modified) flang/lib/Optimizer/Builder/PPCIntrinsicCall.cpp (+3-3)
diff --git a/flang/lib/Optimizer/Builder/PPCIntrinsicCall.cpp b/flang/lib/Optimizer/Builder/PPCIntrinsicCall.cpp
index fcc91752552c3..6090f008adb1c 100644
--- a/flang/lib/Optimizer/Builder/PPCIntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/PPCIntrinsicCall.cpp
@@ -1685,7 +1685,7 @@ PPCIntrinsicLibrary::genVecExtract(mlir::Type resultType,
   if (!isNativeVecElemOrderOnLE())
     uremOp = convertVectorElementOrder(builder, loc, vecTyInfo, uremOp);
 
-  return builder.create<mlir::vector::ExtractElementOp>(loc, varg0, uremOp);
+  return builder.create<mlir::vector::ExtractOp>(loc, varg0, uremOp);
 }
 
 // VEC_INSERT
@@ -1706,8 +1706,8 @@ PPCIntrinsicLibrary::genVecInsert(mlir::Type resultType,
   if (!isNativeVecElemOrderOnLE())
     uremOp = convertVectorElementOrder(builder, loc, vecTyInfo, uremOp);
 
-  auto res{builder.create<mlir::vector::InsertElementOp>(loc, argBases[0],
-                                                         varg1, uremOp)};
+  auto res{
+      builder.create<mlir::vector::InsertOp>(loc, argBases[0], varg1, uremOp)};
   return builder.create<fir::ConvertOp>(loc, vecTyInfo.toFirVectorType(), res);
 }
 

@tblah tblah requested a review from kkwli June 9, 2025 10:01
@dcaballe dcaballe requested a review from joker-eph June 12, 2025 04:34
@DavidSpickett DavidSpickett requested review from ceseo and luporl June 13, 2025 07:37
@dcaballe
Copy link
Contributor Author

kind ping :)

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

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

Look ok to me even I'm not super familiar with the ppc part of flang.

@clementval clementval requested a review from DanielCChen June 16, 2025 18:34
@clementval
Copy link
Contributor

fyi @DanielCChen

@kkwli
Copy link
Collaborator

kkwli commented Jun 18, 2025

I see these LIT failures.

Failed Tests (6):
  Flang :: Lower/PowerPC/ppc-vec-extract-elem-order.f90
  Flang :: Lower/PowerPC/ppc-vec-extract.f90
  Flang :: Lower/PowerPC/ppc-vec-insert-elem-order.f90
  Flang :: Lower/PowerPC/ppc-vec-insert.f90
  Flang :: Lower/PowerPC/ppc-vec-splat-elem-order.f90
  Flang :: Lower/PowerPC/ppc-vec-splat.f90

The error looks like this:

$ cat t.f90
subroutine vec_splat_testi8i8(x)
  vector(integer(1)) :: x, y
  y = vec_splat(x, 0_1)
end subroutine vec_splat_testi8i8

$ bin/flang t.f90
error: loc("/home/kli/t.f90":3:3): 'vector.extract' op operand #1 must be variadic of index, but got 'i8'
error: verification of lowering to FIR failed

@dcaballe
Copy link
Contributor Author

Thanks! Let me fix that. I didn't test with the powerpc target enabled

dcaballe added 2 commits July 7, 2025 17:24
This PR is part of the last step to remove `vector.extractelement` and `vector.insertelement` ops
(RFC: https://discourse.llvm.org/t/rfc-psa-remove-vector-extractelement-and-vector-insertelement-ops-in-favor-of-vector-extract-and-vector-insert-ops)
It replaces `vector.insertelement` and `vector.extractelement` with `vector.insert` and
`vector.extract` in Flang. It looks like no lit tests are impacted.
@dcaballe dcaballe force-pushed the remove-vector-extractelement-insertelement-3 branch from 13243bc to 76c78eb Compare July 7, 2025 18:13
@dcaballe
Copy link
Contributor Author

dcaballe commented Jul 7, 2025

@kkwli, it should be fixed now. Could you please verify? Thanks!

Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

LGTM

It looks like you've addressed comments from @kkwli . I think that you should just land this if there are no further comments by tomorrow.

Thanks Diego!

@kkwli
Copy link
Collaborator

kkwli commented Jul 14, 2025

@kkwli, it should be fixed now. Could you please verify? Thanks!

@dcaballe Sorry I missed it last week. The small reproducer works now but I still see the same LIT failures. For example,

subroutine vec_insert_testf32(v, x, i1)
  real(4) :: v
  vector(real(4)) :: x, r
  integer(1) :: i1
  r = vec_insert(v, x, i1)
end

Now, the code generated is:

define void @vec_insert_testf32_(ptr %0, ptr %1, ptr %2) #0 {
  %4 = alloca <4 x float>, i64 1, align 16
  %5 = load float, ptr %0, align 4
  %6 = load <4 x float>, ptr %1, align 16
  %7 = load i8, ptr %2, align 1
  %8 = urem i8 %7, 4
  %9 = zext i8 %8 to i64   <<<<<<<<<<<<<<<<<<
  %10 = insertelement <4 x float> %6, float %5, i64 %9
  store <4 x float> %10, ptr %4, align 16
  ret void
}

There is an extra zext generated before insertelement. Is it expected?

@kkwli
Copy link
Collaborator

kkwli commented Jul 15, 2025

@kkwli, it should be fixed now. Could you please verify? Thanks!

@dcaballe Sorry I missed it last week. The small reproducer works now but I still see the same LIT failures. For example,

subroutine vec_insert_testf32(v, x, i1)
  real(4) :: v
  vector(real(4)) :: x, r
  integer(1) :: i1
  r = vec_insert(v, x, i1)
end

Now, the code generated is:

define void @vec_insert_testf32_(ptr %0, ptr %1, ptr %2) #0 {
  %4 = alloca <4 x float>, i64 1, align 16
  %5 = load float, ptr %0, align 4
  %6 = load <4 x float>, ptr %1, align 16
  %7 = load i8, ptr %2, align 1
  %8 = urem i8 %7, 4
  %9 = zext i8 %8 to i64   <<<<<<<<<<<<<<<<<<
  %10 = insertelement <4 x float> %6, float %5, i64 %9
  store <4 x float> %10, ptr %4, align 16
  ret void
}

There is an extra zext generated before insertelement. Is it expected?

The extra zext makes sense to me. I create PR #148775 to update the tests. Please go ahead merge it.

Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

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

LG. Thanks.

@dcaballe dcaballe merged commit c99c213 into llvm:main Jul 18, 2025
10 checks passed
dcaballe pushed a commit that referenced this pull request Jul 18, 2025
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 18, 2025
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 19, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-flang-rhel-clang running on ppc64le-flang-rhel-test while building flang at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/34311

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Lower/PowerPC/ppc-vec-splat-elem-order.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o - | /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/FileCheck --check-prefixes="LLVMIR" /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90 # RUN: at line 1
+ /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/FileCheck --check-prefixes=LLVMIR /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90
+ /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o -
/home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90:22:11: error: LLVMIR: expected string not found in input
! LLVMIR: %[[ele:.*]] = extractelement <16 x i8> %[[x]], i16 15
          ^
<stdin>:18:39: note: scanning from here
 %3 = load <16 x i8>, ptr %0, align 16
                                      ^
<stdin>:18:39: note: with "x" equal to "3"
 %3 = load <16 x i8>, ptr %0, align 16
                                      ^
<stdin>:19:2: note: possible intended match here
 %4 = extractelement <16 x i8> %3, i64 15
 ^

Input file: <stdin>
Check file: /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           13:  ret void 
           14: } 
           15:  
           16: define void @vec_splat_testu8i16_(ptr %0) #0 { 
           17:  %2 = alloca <16 x i8>, i64 1, align 16 
           18:  %3 = load <16 x i8>, ptr %0, align 16 
check:22'0                                           X error: no match found
check:22'1                                             with "x" equal to "3"
           19:  %4 = extractelement <16 x i8> %3, i64 15 
check:22'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:22'2      ?                                         possible intended match
           20:  %5 = insertelement <16 x i8> poison, i8 %4, i32 0 
check:22'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           21:  %6 = shufflevector <16 x i8> %5, <16 x i8> poison, <16 x i32> zeroinitializer 
check:22'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           22:  store <16 x i8> %6, ptr %2, align 16 
check:22'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           23:  ret void 
check:22'0     ~~~~~~~~~~
...

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 20, 2025

LLVM Buildbot has detected a new failure on builder ppc64-flang-aix running on ppc64-flang-aix-test while building flang at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/201/builds/5467

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Lower/PowerPC/ppc-vec-extract-elem-order.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o - | /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/build/bin/FileCheck --check-prefixes="LLVMIR" /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90 # RUN: at line 1
+ /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o -
+ /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/build/bin/FileCheck --check-prefixes=LLVMIR /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90
/home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90:30:11: error: LLVMIR: expected string not found in input
! LLVMIR: %[[r:.*]] = extractelement <2 x i64> %[[arg1]], i8 %[[sub]]
          ^
<stdin>:20:19: note: scanning from here
 %7 = sub i8 1, %6
                  ^
<stdin>:20:19: note: with "arg1" equal to "4"
 %7 = sub i8 1, %6
                  ^
<stdin>:20:19: note: with "sub" equal to "7"
 %7 = sub i8 1, %6
                  ^
<stdin>:22:2: note: possible intended match here
 %9 = extractelement <2 x i64> %4, i64 %8
 ^

Input file: <stdin>
Check file: /home/llvm/llvm-external-buildbots/workers/ppc64-flang-aix-test/ppc64-flang-aix-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-extract-elem-order.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           15:  
           16: define void @vec_extract_testi8i1_(ptr %0, ptr %1, ptr %2) #0 { 
           17:  %4 = load <2 x i64>, ptr %0, align 16 
           18:  %5 = load i8, ptr %1, align 1 
           19:  %6 = urem i8 %5, 2 
           20:  %7 = sub i8 1, %6 
check:30'0                       X error: no match found
check:30'1                         with "arg1" equal to "4"
check:30'2                         with "sub" equal to "7"
           21:  %8 = zext i8 %7 to i64 
check:30'0     ~~~~~~~~~~~~~~~~~~~~~~~~
           22:  %9 = extractelement <2 x i64> %4, i64 %8 
check:30'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:30'3      ?                                         possible intended match
           23:  store i64 %9, ptr %2, align 8 
check:30'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:codegen flang:fir-hlfir flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants