This is an archive of the discontinued LLVM Phabricator instance.

[SVE ACLE] Change the lowering of SVE integer builtins
ClosedPublic

Authored by jolanta.jensen on May 15 2023, 2:50 AM.

Details

Summary

Change the lowering of SVE integer mla_x/mls_x and mad_x/msb_x
builtins to use dedicated undef (_u) intrinsics.

Diff Detail

Event Timeline

jolanta.jensen created this revision.May 15 2023, 2:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2023, 2:50 AM
jolanta.jensen requested review of this revision.May 15 2023, 2:50 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 15 2023, 2:50 AM
mgabka added inline comments.May 15 2023, 6:55 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
1666

it is not a bug, but just a preference to keep better ordering here, could you move this new definition to be straight below the def int_aarch64_sve_mla ? so the regular and _u intrinsics are defined in the same order as in all other cases.

Same request to the mls_u definition.

Minor reordering after review comment.

jolanta.jensen marked an inline comment as done.May 15 2023, 8:58 AM
Matt added a subscriber: Matt.May 15 2023, 3:03 PM
paulwalker-arm accepted this revision.EditedMay 16 2023, 5:03 AM

The commit message is incorrect because the SVE Clang builtins are not named mla_u and so should read SVE integer mla_x/mls_x builtins Other than that the patch looks good to me with the following suggestions.

llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith-undef.ll
1307

Just because this is a big file can you follow the existing scheme of sorting alphabetically (i.e. move the new block just before the @mul functions and prefix the two blocks with

;
; MLA
;
;
; MLS
;

accordingly. This just makes it easier to find things when manually scrolling.

1379–1387

As above, please maintain the ordering by moving these declaration just before those for mul.

This revision is now accepted and ready to land.May 16 2023, 5:03 AM

Corrected the commit message.
Added tests are now in alphabetical order.

jolanta.jensen edited the summary of this revision. (Show Details)May 16 2023, 10:12 AM
jolanta.jensen retitled this revision from [SVE ACLE] Change the lowering of SVE integer mla_u/mls_u builtins to [SVE ACLE] Change the lowering of SVE integer builtins.
This revision was landed with ongoing or failed builds.May 16 2023, 10:45 AM
This revision was automatically updated to reflect the committed changes.