- User Since
- Oct 15 2012, 2:12 PM (407 w, 3 d)
I think that we should revert the original patch for now since we have a
known miscompile and a chain of fixes are required to fix it.
*thumbs up on ok for release*
This looks good to me. Thanks for seeing this through Ray!
Wed, Aug 5
Tue, Aug 4
Ow. Can revert and reapply after we fix the caching problem perhaps?
Could maybe add an assert along with the patch as well as an assert only
Tue, Jul 28
I think I'd find the conditionals easier to "read" if they were positive rather than negative? It'd help if the comments spelled out the conditions a little more as well.
Mon, Jul 27
This could probably use some tests. If no current upstream port uses the feature though it makes it hard to accept. :(
Sun, Jul 26
I'd really like all of the cpu specific bits here to not reside in the objdump binary. Ideally it should be in include/ and lib/ somewhere.
Sat, Jul 25
Thu, Jul 23
Fri, Jul 17
(Adding Sterling as well)
I think this will be an OK workaround for now, go ahead and reply on the main thread as I think they'll want to set the default runtime library as part of the toolchain. I'm surprised at this behavior as well.
Thu, Jul 16
Appears so, but ok.
Wed, Jul 15
It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.
We're starting to see miscompiles as we do more testing as well, just nothing smaller at the moment.
This seems like something that we should then revert until we know that instsimplify can be updated with a fix?
Did you want to just change the pentium4 tuning? Otherwise naming things is hard and this feels awkward, but I don't have any better ideas :)
Tue, Jul 14
Mon, Jul 13
Fri, Jul 10
Thu, Jul 9
I may need to comment it better, but in this case part of it is that it's designed to only have the forced loop unroller run on it.
Some inline nits. I see you've already committed and that's fine - I still don't think we should do it, but we can delete it again soon :)
So the tuning here for SCE is also a "does not support" or something else?
Wed, Jul 8
Jul 8 2020
In whose builds and why? What changes would you like? What compiler are you using? Can you provide any more information?
Jul 7 2020
If this works then awesome. :)
Jul 6 2020
Works for me :)
Jul 5 2020
Adding Chandler and Alina here as well.
Jun 30 2020
This causes warnings about -Wc99-extensions due to the use of _Complex.
Jun 25 2020
Committed a while back.
I'm happy with this as well - my only comment was the one Duncan pointed out :)
Jun 24 2020
I think this is the right solution, we need to emit the instruction either way and then hopefully we won't fall out, but if we do we're not leaving mistaken state.
Do we want to actually do the breaking release?
One inline code comment otherwise lgtm.
Jun 23 2020
Jun 20 2020