- User Since
- Jun 28 2016, 8:37 AM (186 w, 5 d)
Thu, Jan 23
This makes a lot of sense to me.
LGTM. note to other reviewers, D72998 fixes the max alignment constant in this.
Tue, Jan 21
Mon, Jan 13
Wed, Jan 8
Sorry, I didn't realize you were waiting for me. This seems fine as it is, I don't see the attribute buying us anything.
Mon, Jan 6
I'm concerned about just making this fallthrough to 'false'. These ARE integral promotions, we just don't know the type size. What happens if you instantiate this template? Will it still give the correct answer/type/diagnosis?
Fri, Jan 3
I don't see anything I have a problem with, but still want @ctopper to have a bit of time to take a look. Ping me monday on both if he doesn't respond.
I think this is alright, I want @ctopper to take a look before I approve it though. Additionally, do you know if this modifies the regcall calling convention at all? Should it?
Thu, Jan 2
Dec 27 2019
Dec 24 2019
Dec 20 2019
@aaron.ballman s comments done.
New patch coming as soon as my build finishes.
Dec 18 2019
Dec 17 2019
Dec 16 2019
Dec 13 2019
Rebase + @eli.friedman s comments.
Sorry in advance for the reviewer choices... Blame got me you all for all touching these things in the early 2010s. Since then, Richard and I seem to be the only one to have touched these types.
Dec 12 2019
Dec 9 2019
I looked at the test failure, the REQUIRES line is necessary because the test calls emit-obj. I've asked Zahira to commit it to fix the bots.
Dec 6 2019
Should this be in a namespace? Can't tell from the header file, is it at least in clang?
Dec 5 2019
Dec 4 2019
Otherwise I think I'm OK. @thegameg ?
Dec 3 2019
Microsoft recently released 16.4 and has ceased support for the 16.3 line, so it isn't really important to me anymore. I'd thought this was a 'hold our nose and do it' type fix, but I don't think that is justified any longer now that Microsoft doesn't even support it.
Dec 2 2019
unique_ptr already has a 'bool' builtin to its state (one that this code checks later anyway), so there isn't a good reason to return a bool here. Otherwise I think I'm OK with this.
Nov 28 2019
Nov 27 2019
Nov 25 2019
Nov 21 2019
I haven't looked at the tests because I don't terribly understand the sanitizer IR (hopefully someone else can take a look), but the logic/motivation seems solid to me.
Nov 18 2019
Nov 5 2019
Oct 11 2019
Oct 10 2019
cant use llvm::Value in Sema without some linker issues.
Remove unused code
Oct 9 2019
Oct 8 2019
Oct 7 2019
Sep 30 2019
Sep 27 2019
Ah, Ugg... GCC must implement the target parsing handling different on a per-architecture basis (ARM vs X86). We've only got parsing in place to do the x86 way. That ends up making this a much more difficult thing to maintain I suspect.
Sep 26 2019
Sep 25 2019
Sep 23 2019
Seems your test changes have disappeared as well? Otherwise, 1 comment then I'm ok with this.
Also, can you add a test for GNUCmpXchg in both situations (inside a template, and outside)? It is the most complicated, and would best reflect the issue.
It kinda stinks that we have to do this at such a late step. I'd have much preferred doing this as a part of the rebuild, but it appears that the 'form' takes quite a bit to calculate. Also, I'd likely have preferred that the initial reordering happened in codegen instead of this early, though perhaps thats too large of a change to do here.
Yikes, good catch!