So, this makes sense to me, but on x86, should we also be worried about the fact that the calling convention is based on which features are available? (>128bit ext_vector_types are passed in AVX/AVX-512 registers, if available). Presumably swift is also affected, no?

Clang/LLVM pieces LGTM. Thanks for doing this, people.

LGTM, thanks!

Ah, that explains it. LGTM, thanks!

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?

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.

Can you add a tablegen testcase?

LGTM without the pattern changes.

Huh, interesting.. Not a big deal, but can you avoid the #ifdef? Where was the overflow?

D32766 replaced this; closing.

Thanks Renato!

In D31642#724540, @chandlerc wrote:

After looking at the uses of AA in both passes here, I think I like the alternative idea of making AA optional directly in the passes. Then, for the old PM, we can use something like getAnalysisIfAvailable and just not mark the analysis as required when not optimizing codegen. What do you think? Would it be helpful if I hack up a patch?

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)

In D32275#732465, @dsanders wrote:

In D32275#732358, @ab wrote:

and also have a test that fails when we add more (maybe something trivial like the number of emitted patterns?)

I'm not sure I understand this bit. Is the intent to prompt us to look at the new rules?

Nice, LGTM!

In D32275#731752, @dsanders wrote:

One thing to mention for this patch is that G_ICMP and G_FCMP are not included due to a difference in operand order between G_ICMP/G_FCMP in GlobalISel and setcc in SelectionDAG (predicate, lhs, rhs vs lhs, rhs, predicate). I can support those in a later patch by adding more information to GINodeEquiv or by changing G_ICMP/G_FCMP.

LGTM

LGTM, thanks for taking care of this!

Makes sense to me. I wish we didn't need this, but progress is made one step at a time

Nice, LGTM

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.

You're right, this is probably the best path forward. A few nits inline

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?

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.

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.

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.

Ping!

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.

• 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.

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)