LGTM, thanks!

A few minor comments inline, and one question: it sounds like this is based on a GCC feature; what's the expected behavior for __builtin_returnaddress ? Shouldn't it xpaci the result?

LGTM. Not the biggest fan of CodeGen/Generic tests, but it gets the job done, I suppose ;) I wouldn't be surprised if it failed on some target that can't even handle half parameters, so you might want to load it instead, put it in x86, or wait and see - your pick.

Precisely: /usr/bin/env is recommended as a cross-platform way to find python. But:

• we're only using lldb on darwin, where we know python (or at least, the xcrun-style shortcut) is in /usr/bin/
• the python interpreter in LLDB comes from /S/L/F, for instance:

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.