Compilers at a fruit company
- User Since
- Sep 9 2013, 3:45 AM (263 w, 21 h)
Thu, Sep 20
@aditya_nandakumar I've added a target hook that defaults to enabling this.
This seems like something that most targets would want done by default. I wonder if we should have a standard set of legalizer actions that targets can use. LGTM anyway.
Mon, Sep 17
This isn't really ideal because of how trivial it is currently. The issue is that if we added the capability to InstSimplify (which is where it would fit most naturally), we'd still end up with the problem of running the whole of InstSimplify in the GlobalISel pipeline. It's reasonable for now I think so I'll commit with the changes requested.
Looks ok, if a bit cumbersome with the additional state (do we need to hold references to it?). I'd like to see how this state is used with the CSE builder first though.
Thu, Sep 13
Ok, I'm surprised that would be a realistic scenario but I'll make the change. Thanks.
Wed, Sep 12
You're right, that's a simplified expression.
Tue, Sep 11
Sat, Sep 8
Wed, Sep 5
Tue, Sep 4
Ok, to me it seems like instructions for which fast math flags can't apply shouldn't be classed as FPMathOperators. insertelement/extractelement are just moving raw bits. If we disallowed those instruction types in the FPMathOperator classof() then this problem should go away, and we don't incorrectly relax FP semantics like in the case I pointed out.
Fri, Aug 31
I’m not seeing how FP instructions can not carry flags. The IR semantics are defined to be strict FP unless explicitly relaxed.
Hit submit too early:
So with the above code B will not have the flag cleared? If that's the case it doesn't seem right to me.
So with this change, if you have:
Thu, Aug 30
So overall I think we should have some form of IR canonicalization before translation for the majority of cases to be handled for -O0.
It can't do that all the time, with these G_ICMPs for example it also has to swap the predicate (not that we even have imported patterns that could possibly match for AArch64).
Wed, Aug 29
I'm not against a separate IR canonicalization stage, even though it's a bit overkill for this particular case. At the moment it looks like InstCombine is doing the canonicalizaton for this case, so re-running that is out of the question. A new pass looks to be on the cards. Anyone else have opinions on this?
Tue, Aug 28
I'd like to get this in. LGTM but needs one issue addressed.
x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?
Aug 21 2018
Looks like Lang added the original code, but LGTM anyway. I even think we did this exact change downstream a few years ago at ARM...
Aug 15 2018
Aug 14 2018
I'll take over and fix this in the way I suggested. Thanks for reporting this!
Aug 10 2018
I'm a little uneasy about specifying libm in the names of the intrinsics. The langref mentions libm in the description, but the semantics don't exactly match due to ignoring errno. That said, I don't really have a suggestion for a better name, so the only thing I can ask is that we make it a bit more explicit in the documentation what we mean by "LIBM" here.
Aug 8 2018
Aug 7 2018
Thinking about this more, I can't think of any reason why we'd ever want to add offsets to an existing offset list. It would probably be better if valueIsSpit cleared the vector (and the doc comment made this clear).
Could you upload with more context please, and a test case?
Jul 31 2018
Jul 30 2018
Jul 29 2018
Thanks for taking this on. I think some x86 tests would also be good here as there evidently isn't much coverage at the moment.
Jul 26 2018
Jul 25 2018
Jul 3 2018
Jul 2 2018
Jun 24 2018
Jun 20 2018
Hi Daniel, sorry for the delay.
Jun 4 2018
Jun 2 2018
Jun 1 2018
May 31 2018
New patch now only zero-extends for stores, removing the use of getBooleanContents.
May 29 2018
I'll re-do this patch to unconditionally zero-extend for returns and also fix up the G_STORE legalisation to always zero-extend too. Given we don't have any target hooks that give us the information we need, let's go with just matching SelectionDAG behaviour?
I got my branches mixed up, this patch is missing test updates. Will upload a new one soon.
May 28 2018
Overall I'm in favour of this change, so LGTM.
May 27 2018
May 23 2018
May 22 2018
Looks like this change caused http://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-aarch64-O0-g/1944/ as well as others.