[PATCH] D91331: Add hook for target to customize different legalization action according to the input type

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added a comment.

In D91331#2419436 <https://reviews.llvm.org/D91331#2419436>, @craig.topper wrote:

> In D91331#2419434 <https://reviews.llvm.org/D91331#2419434>, @steven.zhang wrote:
>
>> In D91331#2419364 <https://reviews.llvm.org/D91331#2419364>, @yubing wrote:
>>
>>> Could we check whether the input type is fp128 at the begining of PPCTargetLowering::LowerFP_TO_INT and if it is a fp128 input type, we directly ConvertNodeToLibcall?
>>
>> I am not sure if this is feasible as ConvertNodeToLibcall is for legalizer not target lower.
>>
>>> Besides, I found a strange thing: In PPCTargetLowering::LowerFP_TO_INT, we will return original op if the input type is fp128, which means in some subtarget , fp128's fptoint is legal and have corresponding instructions. But what you are going to do is to do a hack and expand it to libcall at the beginning of legalizeOp(...). It seems there is a conflict here.
>>
>> That is something that we did to fix the limitation of current setOperationAction mechanism and I am fine if we have any better solution for this... In fact, they are not conflict as the hook means that, expand it as libcall without querying the operation action as the operation action cannot handle the case that there are more than one input type.
>>
>> We used to handle this by having it as custom and inside the hook, different the behavior by different return value. But we have no way to do something like this:
>>
>> Legalize the sint_to_fp as libcall if the source type is fp128.
>>
>> Any thoughts ?
>
> The Expand path falls back to Libcall if the expand fails. So if you return SDValue() from the custom handler, what does the expand for sint_to_fp try to right now for your fp128 cases?
>
> I know do know the Expand for FP_EXTEND will never fail and tries to do a stack memory operation. But maybe we're missing a extload legality check there that we could use to make it fail.

Yeah, we can make FP_EXTEND work if check the legality. For ExpandLegalINT_TO_FP(), it will try to use the stack memory operation to convert the int to f64 first, then, extend it to f128. We also can check the legality of the FPRound/FPExtend to make it fail. Then, we will hit assertion: assert(!isSigned && "Legalize cannot Expand SINT_TO_FP for i64 yet"); So, we can make it fail to if it is not signed. Then, get the final step to have a complicate algorithm to do the convert. But we can make it fail if check the legality.

This is the first attempt I made before but hit too many issues and I feel that the expand will try best to expand it without checking the legality. If we have concern on opening such a backdoor for legalization action, I can update the patch to add the legality check to make the expand fail.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91331/new/

https://reviews.llvm.org/D91331