This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Implement XL compatibility builtin __addex
ClosedPublic

Authored by lei on Jul 28 2021, 1:49 PM.

Details

Summary

Add builtin and intrinsic for __addex.

This patch is part of a series of patches to provide builtins for
compatibility with the XL compiler.

Diff Detail

Event Timeline

lei created this revision.Jul 28 2021, 1:49 PM
lei requested review of this revision.Jul 28 2021, 1:49 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 28 2021, 1:49 PM
lei updated this revision to Diff 362535.Jul 28 2021, 2:05 PM

fix minor wording and spelling mistakes.

lei updated this revision to Diff 362536.Jul 28 2021, 2:06 PM

remove extra space

lei updated this revision to Diff 362734.Jul 29 2021, 5:46 AM

put back unintentional space change

nemanjai added inline comments.Jul 29 2021, 6:00 AM
llvm/lib/Target/PowerPC/P9InstrResources.td
1434

You have added the 64-bit version of this, but it seems this is only available for 64-bit operands in 64-bit mode. Under which conditions do we need the plain ADDEX?

nemanjai requested changes to this revision.Jul 30 2021, 5:55 AM

Requesting changes until my comment is answered/addressed.

This revision now requires changes to proceed.Jul 30 2021, 5:55 AM
lei added inline comments.Jul 30 2021, 6:49 AM
llvm/lib/Target/PowerPC/P9InstrResources.td
1434

Correct, the builtin __addex() is only available for 64bit.
Unless I am reading the ISA wrong, the instruction addex is valid for both 32bit and 64bit:

ForCY=0,OVis set to 1 if there is a carry out of bit 0 of the sum in 64-bit mode or there is a carry out of bit 32 of the sum in 32-bit mode, and set to 0 otherwise. OV32 is set to 1 if there is a carry out of bit 32 bit of the sum.
NeHuang added inline comments.Jul 30 2021, 11:51 AM
clang/lib/Sema/SemaChecking.cpp
3426

I think we start using isa-v30-instructions for pwr9 only (or later process) in SemaFeatureCheck

clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-warning.c
5

can we also add the run lines for 64 bit LE Linux, 64 bit AIX and 32 bit AIX? Will also need #ifdef __PPC64__ for the test case.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-pwr9-64bit.ll
44

This test case is identical as call_addex_0 The unsigned long long and signed long long scenarios produce identical IR and we should only keep one test case here.

lei updated this revision to Diff 365480.Aug 10 2021, 8:05 AM
lei marked 2 inline comments as done.

update sema check condition and remove duplicate tc

lei added inline comments.Aug 10 2021, 8:06 AM
clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-warning.c
5

AFAIK there is no difference between LE and BE for sema checking and it'll be overkill to add the ifdef since the only run line in here is for 64bit compilation. I think that can be added later if there are needs for 32bit testcases later?

nemanjai accepted this revision.Aug 10 2021, 2:50 PM

LGTM.

This revision is now accepted and ready to land.Aug 10 2021, 2:50 PM
NeHuang accepted this revision.Aug 11 2021, 5:50 AM
lei updated this revision to Diff 365816.Aug 11 2021, 12:04 PM

Add -W flag to new warning message

lei updated this revision to Diff 366062.Aug 12 2021, 11:54 AM

Update diag id

stefanp accepted this revision.Aug 12 2021, 12:09 PM

Thank you for adding the DiagGroup.
LGTM.

lei updated this revision to Diff 366094.Aug 12 2021, 1:53 PM

Fix name of new warning message to be more accuratly represent the diagnostic.

This revision was automatically updated to reflect the committed changes.