Custom lower f128 setcc/select_cc as libcall if power9 vector is not enabled.
Details
- Reviewers
nemanjai jsji steven.zhang - Group Reviewers
Restricted Project - Commits
- rGece7345859c3: [PowerPC] Lower f128 SETCC/SELECT_CC as libcall if p9vector disabled
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Please explain in the description of the review (and subsequent commit message) why custom lowering is needed. I suppose for SELECT_CC, it is needed because using setOperationAction() uses the result type which is only guaranteed to match the true/false value but not the comparison types. However, it is not immediately obvious to me why we need custom lowering for BR_CC.
Update the description. The reason why we need to custom lower the BR_CC has been explained in the code comment. The legalizer tries its best to expand the BR_CC which is not what we want.
// Lower the br_cc as follows for fp128 as we didn't have native // instruction to lower the setcc on fp128. // br_cc cc, x, y, dest -> // z = setcc x, y, cc (expand as libcall) // br_cc NE, z, 0, dest
The description isn't really adequate - it does not answer my question at all really. The question is why not set the legalization action for BR_CC as Expand? Namely, what does the legalizer do that we don't want it to do if we let it expand the node? You just answered the question with the equivalent of "The legalizer does its job and that's not what we want." So please explain why that is not what we want.
Hmm, we will hit assertion with legalizer as it tries to swap the operand and cc to see if target support it, and assertion otherwise. Basicly, we have two options. One is to enhance the legalizer and another one is to do it inside PowerPC target as what this patch did. I am swing between them. Thank you for bringing this up and I will update the patch to go with direction on enhancing the legalizer and it might answer your question directly.
Split the BR_CC into another patch which will enhance the legalizer to support it. For SETCC/SELECT_CC, we will do it inside PowerPC as what other target did.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
1235 | Please confirm if there is any problem with the pattern like: select_cc f128, f128, v2f64, v2f64. Technical speaking, we need to custom all the legalized type. |
Mark BR_CC as expand.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
1235 | We don't need to mark vector types for select_cc here. Instead, select_cc f128, f128, v4i32, v4i32 would crash because of br_cc not expanded. |
Will this okay if IsStrict == true?