- User Since
- Nov 17 2016, 12:59 PM (200 w, 6 d)
Tue, Sep 22
Mon, Sep 21
Fri, Sep 18
Thu, Sep 17
Updated diff LGTM
Wed, Sep 16
I pretty much only have comment nits at this point, so I think this LGTM.
Tue, Sep 15
In the future, can you do a full-context diff? It lets reviewers see the full file.
Mon, Sep 14
Remove assert because I forgot that there are a million different generic load instruction variants
Pass the opcode table as a parameter to a helper function
Fri, Sep 11
Wed, Sep 9
Tue, Sep 8
Fri, Sep 4
LGTM now that I realized I just misread the code here
For AMDGPU this would need a regbank legality check for post-regbankselect combines but I'm not sure what to do about that
Thu, Sep 3
Oh wait the tests already test what I was concerned about. LGTM with minor nits.
Is there any way to test the hash itself on certain instructions?
Tue, Sep 1
Personally I can't see anything wrong with this at this point.
Thu, Aug 27
LGTM, assuming the pre-merge check thing is just being wonky and m_GXor exists.
Aug 24 2020
Shouldn't running the copy_prop combine post-legalization do the same thing as this?
Aug 19 2020
Oh whoops, sorry! Landing now.
Aug 18 2020
Aug 17 2020
Aug 14 2020
LGTM with a couple nits on comments
Aug 11 2020
I applied D85201 and the tests passed without any conflicts/updates, so it seems fine.
- Add back tests
- Use = default for default constructors
- Add some asserts to applyBuildInstructionSteps
Aug 10 2020
- Use a more functional style + generalize a bit more so this can be used for other combines
- Allow vectors + update test
- Add a helper function for legality checks
Aug 7 2020
Forgot to type "differential revision" in commit, closing.
Aug 6 2020
Also yeah, it looks like this is handled in SelectionDAG::GetDemandedBits:
I didn't port this directly from the DAGCombiner, I originally wrote it just to handle the G_ICMP + G_AND pattern, which appears fairly often in AArch64.
Could we have more than 2 possible choices for opcodes? Would it make sense to make this take an arbitrary number of alternatives?
Aug 5 2020
Aug 4 2020
Aug 3 2020
Jul 31 2020
This has been sitting for a while, and I can't see anything wrong with it. I think @arsenm's justification makes sense, so I'm just going to go ahead and give this a LGTM.