- User Since
- Aug 18 2015, 2:21 AM (294 w, 5 d)
Feb 9 2021
Fix an assertion error caused by attempting to get the frame size of non-frame instructions
I've discovered an issue, this is not yet ready for review.
I have applied the code review suggestions and committed this to master in 2ccb941740e608a9cf70a7c5840497149654b0f6.
Feb 4 2021
Feb 1 2021
Jan 28 2021
Jan 27 2021
Looks good to me. Indeed, it is sad this could not be implemented at a higher level but even as is it is a great optimization.
Jan 23 2021
Nice work, approved.
Nov 18 2020
Nice patch, cheers.
Nov 17 2020
@arichardson yup, this is good to go. Thanks for the persistence on this one.
Nov 16 2020
Looks good to me, thanks for the patch!
Looks good to me, thanks for the patch @benshi001
Oct 28 2020
Approved, thanks for the patch!
Regarding TableGen; I would like to send an RFC to llvm-dev to come up with a proper API to expose backend-specific device-specific information and constants to LLVM frontends, as I can imagine that many backends could stand to benefit. I note that there is in general a lot of duplication in individual frontends defining information LLVM already knows about (off the top of my head, inline assembly constraints and registers is one of them but may not easily fit into the paradigm I have in mind).
Oct 27 2020
Sep 30 2020
Good bugfix - thanks for the second patch!
The code is looking nice, thanks for the patch @amikhalev!
This is a great patch, thanks @couchand!
Sep 21 2020
Hey @aykevl, could you please confirm that this patch looks okay?
Aug 27 2020
Note to the readers: A recent-ish llvm-dev discussion around this patch can be found in here whrein a consensus is found on the way forward on this patch.
Aug 26 2020
I've split the backwards compat tests and AMDGPU changes out to D84345. Is the DataLayout upgrade what you meant with bitcode compatibility tests or should I be adding additional tests?
Aug 24 2020
Aug 11 2020
Looks good, thanks for the patch. Do you need someone to commit this for you @ixoo?
Patch is looking really good, only suggestions are the adding a test and a couple formatting nitpicks.
Jun 23 2020
Thanks for the patch @rodrigorc, appreciate it!
Jun 19 2020
Rebased version with @Sh4rK's comments fixed.
@Sh4rK your review is always welcome - I've updated the code with your comments, here's the delta containing just the fixes you've suggested.
This conflicts with master now, he's a rebased patch
Jun 18 2020
Sorry, I lost track of this. Committed in 01741d6dbec11c0a0c8e610f0033831735c78d1e
Note: I don't know whether the way I've made the Z register printable is the correct way. Maybe the register should be added to the MCInst instead.
Good reproduction, works exactly as expected and indeed this value is being miscalculated.
Jun 17 2020
Couple of nitpicks, otherwise good to go.
Jun 16 2020
This is good, this is good. Nice work @aykevl, approved.
Added an integration test case
Jun 14 2020
Also, you may already know about it but if not, the LLVM bugpoint tool can usually heavily reduce testcase size. I usually get 99% cutdown from some of the larger IR files. If the file must be large though, it must be large (as some tests are) so let's add it anyway.
Ok, I can add the test case (need to recompile LLVM first). It just seems a bit large to me (over 1000 lines).
Jun 8 2020
May 31 2020
I've checked over all of the relocation logic, cross referenced with both the AVR ISA manual and the AVR LLVM fixture implementations in AVRAsmBackend.cpp - everything looks consistent to me.
Are you planning on adding these integration tests to the LLVM buildbot or something more manual?
Added an integration test for this and verified that the test does indeed miscompile and crash the AVR simulator before the patch, but work correctly afterwards. The testcase output is also consistent with AVR-GCC.
May 30 2020
May 17 2020
@LemonBoy merged in into master in 1335737ee119b59bc25ee6d1bb200cf3b975e196
@vlastik Merged into master in 1420f4efbe7ca34355b2dd85d396197d92cd5e3f.
May 16 2020
A thought: should the AVR family be included in the target triple? Like avr5-unknown-unknown (similar to armv6m-none-eabi, armv7m-none-eabi, etc).
The different variations do sometimes change the architecture in incompatible ways, such as with the call instruction which pushes a return address of two or three bytes depending on the architecture variant (and thus makes it impossible to pass parameters on the stack reliably).
Good question, I wasn't able to find any document online.
All the info I've used come from this ISA manual and by feeding some asm into ld to see if the result matched my expectations.
I really like the idea behind this patch, but I suspect there is a better way to go about it.