This is an archive of the discontinued LLVM Phabricator instance.

[flang] Add PPC vec_add, vec_and, vec_mul, vec_sub and vec_xor intrinsics
ClosedPublic

Authored by kkwli0 on May 31 2023, 4:41 PM.

Details

Diff Detail

Event Timeline

kkwli0 created this revision.May 31 2023, 4:41 PM
kkwli0 requested review of this revision.May 31 2023, 4:41 PM

Please ping the reviewers when the CI build problem(s) is resolved.

Please ping the reviewers when the CI build problem(s) is resolved.

Thanks. Will do.

vzakhari added inline comments.Jun 5 2023, 10:28 AM
flang/include/flang/Optimizer/Builder/IntrinsicCall.h
92

typo: // -> ///

flang/lib/Optimizer/Dialect/FIROps.cpp
1009

Please use free function variants for isa (see https://reviews.llvm.org/D151556 for details).
Don't you also need to call canBeConverted for the element types of the vector types?

flang/module/__fortran_ppc_intrinsics.f90
34

Does Flang support unsigned?

klausler added inline comments.Jun 5 2023, 10:40 AM
flang/module/__fortran_ppc_intrinsics.f90
34

Yes, but only for PPC vectors.

kkwli0 marked 4 inline comments as done.Jun 7 2023, 7:49 AM
kkwli0 added inline comments.
flang/lib/Optimizer/Dialect/FIROps.cpp
1009

Will add the check for the element types as well as size.

kkwli0 updated this revision to Diff 529427.EditedJun 7 2023, 2:25 PM
kkwli0 marked an inline comment as done.

Changes in this update:

  • rebase
  • fix the CI build breakage
  • enhance checking in conversion between fir vector and mlir vector
kkwli0 updated this revision to Diff 529487.Jun 7 2023, 7:40 PM

Change

  • fix format
  • remove unused headers
klausler added inline comments.Jun 8 2023, 7:21 AM
flang/lib/Semantics/resolve-names.cpp
4855

It would be more informative to be clear that the vector type is only available on PowerPC.

DanielCChen added inline comments.Jun 8 2023, 8:09 AM
flang/lib/Semantics/semantics.cpp
523

This seems no longer needed as it is checking the target from targetCharacteristics.

kkwli0 added inline comments.Jun 8 2023, 11:57 AM
flang/lib/Semantics/resolve-names.cpp
4855

I will change the message to "Vector type is only supported for PowerPC."

flang/lib/Semantics/semantics.cpp
523

Yes, it is no longer needed. I will remove it. Thanks.

DanielCChen added inline comments.Jun 9 2023, 8:32 AM
flang/lib/Semantics/semantics.cpp
526

Actually, this checking is not needed also as the __fortran_ppc_types will always built.

kkwli0 marked an inline comment as done.Jun 9 2023, 8:42 AM
kkwli0 added inline comments.
flang/lib/Semantics/semantics.cpp
526

Yes. That module is always built. It does not need the check. I will remove the check. Thanks.

kkwli0 updated this revision to Diff 530534.Jun 12 2023, 9:09 AM
kkwli0 marked an inline comment as done.

Rebase and address review comments.

kkwli0 updated this revision to Diff 530618.Jun 12 2023, 11:40 AM

Fixed CI build failures

jeanPerier accepted this revision.Jun 13 2023, 5:22 AM

Thanks, this looks great.

flang/lib/Optimizer/Builder/IntrinsicCall.cpp
121

Nit: MLIR is moving to mlir::isa<mlir::Float32Type>(eleTy). Also applies to isFloat64.

167

Nit: no braces in single line for in lowering code

4666

nit: single line if.

This revision is now accepted and ready to land.Jun 13 2023, 5:22 AM
kkwli0 marked 3 inline comments as done.Jun 13 2023, 10:08 AM
kkwli0 updated this revision to Diff 530984.Jun 13 2023, 10:43 AM

Address review comments

Thanks for reviewing the patch.