In D72032#1800035, @dblaikie wrote:

I take it there's no way to write a portable test for this patch? (one that doesn't have to run on the specific hardware, but still demonstrates the presence of this newly added support?)

Thanks. I'll wait for the other patch to be submitted and then change the binding.

OK great.

That is weird. If I understand the intel doc correctly, both these names refer to the same event (EventSel=A1H, UMask=01H). I have a haswell machine and dispatched worked (but I patched your change and I get the exact same results, which is to be expected given that the event is the same). What version of libpfm do you have ? I've seen both work on`4.10.1+git14-g815ff28-1`.

This makes the code easier to read indeed, but this comes at the expense of allocations and less locality, so I would refrain from doing this unless we have a good reason.
Or: gather some data in the codebase as to whether a template-parametric SmallBitVector would find other usages, implement it, and use it here as a 3rd parameter to PackedBitVector.

LGMT from the exegesis side. Disclaimer: I know nothing about MIPS.

Please also change the title of the patch, to avoid panic :)

More comments.

And add unit test.

In D70874#1764446, @craig.topper wrote:

I'm not sure I understand why this needs to be initialized. Why don't we need to do it for FPCW?

In rG95fe54931fddccccf9740b3247219e30504da447#855727, @Jeroen wrote:

Does it make sense of extending the unit test with auto, auto & and auto && examples for the variables?

In D60318#1756381, @RKSimon wrote:

@courbet what's happening with this patch?

Thanks for the review.

Use LLVM style

Thanks for the review.

Use the right triple for big-endian thumb.

Address comments.

Rebase on thumb tests.

Address comments.

When zero-extending, check the legality of the zero-extended load.

Be conservative on ARM and do not introduce illegal BSWAPs when zero-extending.

rebase on NFC change

Rebase to show test diffs.

Remove && warnings.

In D70390#1751751, @Quuxplusone wrote:

that sounds like a big non-local change whose correctness would be very difficult to verify.

I'm going to remove the && warning from the check and keep only the const one, as I think these is a consensus on that.

IMHO these two should just not overlap. It makes sense, to have controversial or configurable stuff in clang-tidy. It should just be consistent with the warnings, as those are "always right" and clang-tidy can be opinionated/specialized.

Adding @Quuxplusone (warning author) for opinions.

Actually, thinking more about this, adding this to the existing warning might not be a very consensual change. In the case of the warning:

Address comments.

Thanks.

Fix markdown in doc.

Thanks for the comments

No, we only have SSE2 and above.

In D69044#1728571, @courbet wrote:

I don't remember cases where we had very large constant compares (though we do have quite a lot of small ones). I'll run our internal benchmarks with this change.

I don't remember cases where we had very large constant compares (though we do have quite a lot of small ones). I'll run our internal benchmarks with this change.