Skip to content

[mlir][Flang][NFC] Replace use of vector.insertelement/extractelement #143272

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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?

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-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 luporl and ceseo 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants