Page MenuHomePhabricator

[PowerPC][FP128] Fix the incorrect calling convention for IEEE long double on Power8
ClosedPublic

Authored by steven.zhang on Nov 16 2020, 3:21 AM.

Details

Summary

For now, we are using the GPR to pass the arguments/return value for fp128 on Power8, which is incorrect. It should be VSR. The reason why we do it this way is that, we are setting the fp128 as illegal which make LLVM try to emulate it with i128 on Power8. So, we need to correct it as legal.

If the type is illegal, LLVM tries to soften the type with libcall. However, if the type is legal, we have to convert it into libcall inside LegalizeOp() which is a complete different code path with old implementation. I see quite a lot of cases failures due to different reason(some might be PowerPC target issue, some are legalizer issue). So, I added a temp option and guard the fix under that option and fix these issues with several patches. The temp option will re moved if all the issues fixed.

Diff Detail

Event Timeline

steven.zhang created this revision.Nov 16 2020, 3:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2020, 3:21 AM
steven.zhang requested review of this revision.Nov 16 2020, 3:21 AM
qiucf added a comment.Nov 16 2020, 4:44 AM

Can you test if this patch makes P8 fail the same number of cases as P9 under -mabi=ieeelongdouble in LLVM test suite? Thanks.

Can you test if this patch makes P8 fail the same number of cases as P9 under -mabi=ieeelongdouble in LLVM test suite? Thanks.

Sorry, I cannot. Because we just make several opcodes work inside this patch. There are other opcode that depends on legalizer's work. So I split them. We can do this test when I remove the temp option.

qiucf added inline comments.Nov 23 2020, 12:12 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
124

opton -> option

126

will it be better if using enable-soft-fp128 instead of specifying the target is P8?

1169

can we common some code in previous if and this else? I see some ops are also marked Expand under P9.

Address comments.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
124

Good catch.

126

Nice rename !

1169

I will common the code when removing the EnableP8FP128 options. So that, it is easier to do the code review. It is ok ?

nemanjai accepted this revision.Nov 24 2020, 3:38 AM

LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
1169

Yes, I'd rather keep the code guarded by this temporary option completely separate. There is some code duplication, but it clearly separates this code to make it obvious what we do for this case.

This revision is now accepted and ready to land.Nov 24 2020, 3:38 AM