User Details
- User Since
- May 15 2018, 2:45 AM (140 w, 2 d)
Yesterday
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.
Address comments.
LGTM.
Tue, Jan 19
Mon, Jan 18
Gentle ping...
Sun, Jan 17
@nemanjai Do you have any more comments on this ? Thank you!
Ping...
Tue, Jan 12
Thank you for doing this. LGTM now.
Mon, Jan 11
This is the same as what I want to do in D86681. As D86684 is still pending, it makes sense to split from it now.
LGTM. Please hold on for days to see if @nemanjai has more comments.
Sun, Jan 10
Rebase the patch.
Ping ...
LGTM, but please hold on for days to see if @craig.topper or @nemanjai have more comments.
Thu, Jan 7
It surprises to me that you didn't see the difference between LE and AIX, which doesn't make sense to me. Please double confirm to see if there is any conflict when update the test with the script.
Wed, Jan 6
Tue, Jan 5
Gentle ping ...
Hmm, so we can try to remove those parts that not relative with the parameter arsenm mentioned. I guess most of the check in the DAGCombiner could be removed if I understand correctly. To remove it completely, some work is needed for parameters or others that current IR cannot represent the semantics of the global setting. Is it right ?
- Reason why we need to change the interface
Ping...
Mon, Jan 4
Some code style comments.
Wed, Dec 30
Also LGTM.
Tue, Dec 29
Mon, Dec 28
Gentle ping...
Sun, Dec 27
Is it better to have the check before 'if (PDT->dominates(To, BB)) {' ?
Rebase the patch again.
Re-implement with SelectionDAGTargetInfo.
Thu, Dec 24
The dependent patch changed. Need rebase.
Wed, Dec 23
Thank you for the review and Merry Christmas !
Address comments and add tests in llvm/test/CodeGen/PowerPC/constant-pool.ll to show the padding added for alignment. Also, add the RUN line for Power8 as requested.
Tue, Dec 22
Yes, test is missing as MaskRay mentioned. You'd better update one PowerPC test at least so that, we can see what it looks like.
Dec 21 2020
The test you are looking for is llvm/test/CodeGen/PowerPC/constant-pool.ll in fact.
Rebase the patch.
Dec 17 2020
The new added field LayoutIndex miss to be initialized in include/llvm/ProfileData/SampleProfWriter.h:281 which cause the compiler(clang) warning if with option -Wmissing-field-initializers.
This is the error message:
error: missing field 'LayoutIndex' initializer [-Werror,-Wmissing-field-initializers]
Dec 16 2020
That means we have to make it as target independent. I will take a look to see if there is nice way to make it as target independent.
Address comments.
Rebase tests change for PowerPC.
So, is this patch still incomplete as I didn't see the test change ?
Dec 15 2020
LGTM now. But hold on for some days to see if @nemanjai has more comments. We need some follow up to revisit the floating point instructions and set the sideeffects flag accordingly before they are modelled in fact.
LGTM now. But please hold on for several days to see if Stepfan or others have more comments.
Dec 14 2020
Gentle ping ...
Dec 13 2020
Dec 11 2020
LGTM as it makes sense. But hold on for several days to see if others have more comments.
Dec 9 2020
Address comments.
Dec 8 2020
As we have the time pressure for this, if there is concern from other target about this patch, I will abandon this patch and do it inside PowerPC as what AArch64/X86 did now. Personally, I like the idea to common them into legalizer.