- User Since
- Apr 11 2016, 3:46 AM (97 w, 4 d)
I fixed all issues raised by Gerolf except 3 questions: I put my answers/comments on them.
I think the right solution is to tell CheckImplicitConversion that it's not just a straight-up conversion, it's a conversion done as part of a compound assignment, and then CheckImplicitConversion can choose to suppress or use different diagnostics based on that information. This will also more reliably avoid duplicate diagnostics.
Wed, Feb 21
I re-based the sources and added a new test to demonstarte new debug output.
Tue, Feb 20
I was going to commit it as a part of D40602 because the isue raised there. Is it OK?
Wed, Feb 14
MachineCombiner part LGTM now. I still would prefer if you submit it without the change in lib/CodeGen/MachineInstr.cpp and do that in a separate change.
- The issue with "\n" was fixed.
- About pattern ID in output: in theory there could be several patterns for one Root and every pattern could have several alternative sequences, right? I think in this case the pattern ID will help to distinguish all these removed/substituted sequences from each other. Am I right or it's better to remove the IDs?
Tue, Feb 13
I removed the ambiguity with changes in CheckImplicitConversion (required by rksimon): now it's absolutely clear what was changed.
And I minimized changes in diagnostics(required by rjmccall): the only float-double truncation was changed. As result only two tests were changed. Later we could easily extend the diagnostic updates if it will be necessary (but inside another patch).
Mon, Feb 12
Fixed by 324721.
fhahn, do you have any questions here?
Fri, Feb 9
Committed revision 324721.
BTW, could you review D42728: it's rather similar to this one and rather small as well.
Thu, Feb 8
I propagated qualifiers accordingly to rjmccall's suggestion. But I changed the diagnostic: now it's more realistic from my point of view.
Tue, Feb 6
Fri, Feb 2
I re-implemented the patch. Now it works properly with const methods in CXX classes.
Thu, Feb 1
In fact we have here another problem: it is not an attempt to assign to const variable but it is an attempt to use the field assingment inside const method: I'm working on it.
Wed, Jan 31
I removed incorrect output about possibly removed instr: instead I added an output about Patterns used in combiner.
Fri, Jan 26
Thu, Jan 25
Jan 19 2018
I removed the issue with BDVER1 tests. Now we have one diff for GENERIC CPU but I'm sure it's OK.
Jan 18 2018
I added special switch "mc-shld-enabled" to allow the job of this patch. As result we have only one changed test specially added by me previously. And now the whole patch is rather small.
I bootstrapped the compiler with this patch but unfortunatley there was not any machine-combiner activity. It means we need special test(s) to validate the given substitution.
Jan 17 2018
Jan 16 2018
Jan 15 2018
Jan 12 2018
I removed WriteVecLoad as RKSimon required: it did not have any influence on cost numbers.
I removed all changes related to debug logging: they are published as D41278. In addition I completely implemented the possible substitutions for SHLD64mri8, SHLD64rrCL and SHLDrri8. I did not implement the same for SHLDmrCL because from my point of view the alternative code sequence is too long. Maybe I'm wrong?
The name of the new switch was changed.
Jan 11 2018
Jan 10 2018
RKSimon, do you like to see here anything more?
Following requirements I got rid of class attribute MF and replaced it with STI. In addition I added special hidden switch to allow dump of substituted instructions.
I removed usage of MRI thanking to Craig's suggest.
Jan 9 2018
I renamed the feature and replaced the assertion with dyn_cast.
The implemetation was switched to lambda function.
Dec 29 2017
Dec 28 2017
I changed the implementation accodingly Craig's requiremetns: now it's shorter and more effective.
Dec 27 2017
Dec 26 2017
Dec 25 2017
I changed lattency for MOVSS/MOVSD and MOVAPS/MOVAPD and MOVDQA/MOVDQU.
I fixed the comments raised by fhahn except the one: I can't show the new output format just now because we don't have real Machine Combiner Patterns tests at the moment. When I commit D40602 I'll be able to prepare such tests.
Dec 21 2017
I removed everything related to WriteVecMove and WriteVecStore.
Dec 17 2017
I removed all changes related to Atom.
Dec 15 2017
The typo was fixed.
Dec 14 2017
The latest Simon's comments were implemented.
It seems all requirements from Simon were satisfied.
Dec 12 2017
All comments from Simon were resolved. There was a real bug and it's fixed in this new patch - tnx, Simon.
Dec 11 2017
Dec 7 2017
Dec 6 2017
Nov 29 2017
Nov 28 2017
Nov 27 2017
I fixed all Simon's notes/comments/requirements.
Nov 25 2017
Nov 23 2017
Last Simon's comments were fixed.
Nov 22 2017
I removed schedule-x86-64-shld.ll.