In D151092#4368151, @aaron.ballman wrote:

The release note references documentation that doesn't exist, so Sphinx is failing: https://lab.llvm.org/buildbot/#/builders/115/builds/46942

address review comments

fix release note placemenfix release note placement

Let's submit this quick fix first and we can re-enable it after your other patch.

I wanted gchatelet@ to have a look at this, because he wrote the Instruction/Operand code. In particular, I'm wondering why we're not using the info on the actual Operand value rather than relying on the MCOperandInfo for the operand. Specifically, we do know that this is a register since we return true for isReg, so we could just bake this knowledge into getExplicitOperandInfo in a target-independant way.

Thanks for the review.

Thanks for the data. Can you get someone to approve that the size change is OK ? Otherwise LGTM for the exegesis part.

Fix release note placement and phrase it more consistenly with other notes in that section.

• Address review comments
• Add release notes.

ping

No strong opinion on this, if you feel that's better that's fine with me.

Thanks for the patch.

In principle I like that, however this has implications on the size of the various llvm binaries. Can you please measure the increase in binary size for e.g. clang ?

This is great, thanks for the patch.

Thanks. In principle that sounds goods, my only real remark is about what flags we use to communicate that intent to the tool.

I don't think JIT'ing X86 modules on SystemZ is supported.

This one is NFC, tag it as such ?

add matcher to registry and add release note

In D141925#4073385, @aaron.ballman wrote:

You also need to add the new matcher to Registry.cpp so it's usable from the dynamic matchers, and you should add a release note for the new matcher.

ping

Regenerate doc

fix doc

fix ddoc

In D140271#4056430, @lebedev.ri wrote:

In D140271#4056202, @courbet wrote:

Are you seriously saying that 10x wallclock improvement is negligible?

10x speed improvement is not negligible. I'm simply questioning whether speed matters in this case.

If the speed improvement came at no cost, that would be a no-brainer. But there is a speed/readability tradeoff, which we need to evaluate.

All the changes here are to a single function,
that isn't really going to change further anyway.
It's not like this requires changes to many places.

Are you seriously saying that 10x wallclock improvement is negligible?

Do we really need the speed boost ? My worry is that we are making the code more complex for a negligible advantage. I personally have never felt that waiting for a couple of minutes was really an issue, but you might have other use cases in mind ?

I think we should keep a single one, but just to bikeshed, a TypeSize has a Value, not a Size, so I would keep getFixedValue.

Nice, thanks.

In D140702#4024640, @lebedev.ri wrote:

To be noted, i feel like it's a moot point to argue over the namings. It's mostly irrelevant in the long run.
What do @courbet @RKSimon think? I can change it any way that is agreed.

The code in this patch looks good to me, and I think it turned easier to read than the LinearPoly* abstractions. So unless there is a good reason to keep them, LGTM.