- User Since
- Oct 15 2014, 9:44 AM (178 w, 6 d)
Jan 4 2018
Clang/LLVM pieces LGTM. Thanks for doing this, people.
Dec 9 2017
Dec 7 2017
Nov 17 2017
Nov 9 2017
Sep 18 2017
Sep 12 2017
Aug 17 2017
Ah, that explains it. LGTM, thanks!
Aug 10 2017
Jul 27 2017
Jul 5 2017
There's a test for the call side in r294970, and I added some tests in r299093, you can probably make either check the generic fastisel fallback diagnostic too?
Jun 27 2017
This looks OK too.
Can you emit a single table for all patterns? I'm fine with doing that separately; looks OK otherwise.
Can you add a tablegen testcase? If nothing else, it's a good way to document the tablegen backend.
With the non-optimization comments addressed, I'm OK with this.
Jun 26 2017
Jun 16 2017
Jun 15 2017
Can you add a tablegen testcase?
LGTM without the pattern changes.
Jun 2 2017
May 15 2017
Huh, interesting.. Not a big deal, but can you avoid the #ifdef? Where was the overflow?
May 9 2017
D32766 replaced this; closing.
May 3 2017
May 2 2017
This looks fine; the TLI unittest is the best place for testing this. (for instance, no-proto.ll wouldn't help, because the pass doesn't know about these libfuncs and can't modify them anyway, regardless of the prototype)
May 1 2017
Apr 26 2017
Apr 25 2017
Apr 21 2017
Apr 20 2017
Apr 19 2017
Apr 17 2017
Apr 14 2017
LGTM, thanks for taking care of this!
Apr 13 2017
Makes sense to me. I wish we didn't need this, but progress is made one step at a time
Thanks folks! No objections for D31303 then?
16-bit is weird, but it'd be great if you could add a testcase for that.
Nice catch, LGTM.
Apr 12 2017
You're right, this is probably the best path forward. A few nits inline
Apr 10 2017
I'm slightly worried that this makes the selector functions a bit harder to read, but this seems like a nice simplification otherwise.
Does this really depend on D31329? Can you invert the dependency and put the tblgen emission bits in that patch?
Apr 6 2017
I'm not a big fan of this approach (because the iterative logic seems like it does belong in the helper), but it's mostly fine (we don't have users other than Legalizer), and it sounds like Tim and Quentin are on board. So, LGTM.
LGTM after removing the whole check and adding a good explanation in the commit message.
Apr 4 2017
Apr 3 2017
Of course, sorry, here's some context: Michael and I noticed 4 unnecessary DT computations at O0. Altogether, they added up to a non-trivial compile-time win on the CTMark subset of the test-suite: 1.3-4.7%, 2.5% geomean.
Mar 31 2017
This seems to only add optional uses, what about OptionalDefOperand? (I'm fine with "we don't need that", but I don't see anything that would make the import fail)
Assuming Kristof and Diana agree, LGTM with Diana's comments (and a couple more of my own) addressed.
Mar 30 2017
I considered and dismissed speculatively printing: it's a pretty expensive operation (especially when it needs to print an IR value, and it needs to recompute slot numbers).
Nice! LGTM with the header changes.
Mar 29 2017
- Remove -fast-isel-verbose
- Move ORE make_unique up
If a function always returns an Error, what does it return on success? It would be strange to have an Error that is not an error.
Mar 28 2017
Can you explain why this is necessary?
Merge pain is temporary, but commit history is forever (I spend a lot of time resolving conflicts, but I spend way more git-blaming ancient commits). So, I'd favor doing the right thing in the first place if it's not too painful (maybe try it and abandon if it takes more than a few minutes to resolve? I suspect you'll have few problems as the header isn't affected by most changes)
Mar 27 2017
- Tweak parameter names
- Add a getORE method