Looks reasonable to me. I'd have probably done something like:

Removed errant "const".

LGTM!

(assuming the LIT failures are fixed of course!)

Nice cleanup, thanks!

In general anything moving from 32-bit to 64-bit bitmasks in the backend LGTM. I'd even go further and suggest using std::bitset to remove this problem Once And For All, but that would require significantly more benchmarking.

In fact, one concrete improvement we could do with this CL (which clearly found everything we needed to patch) is introduce a typedef for "A bitmask of functional units". This would allow us to find all the instances again in future, and highlight the point of the bitwidth to users.

LGTM, sorry for the delay.

Hi Alex,

LGTM, but in which cases are we enforcing const-correctness? const-correctness is not a property LLVM enforces almost anywhere because of its virality.

Thanks Thomas!

LGTM, thanks!

Updated based on David's comments.

Thanks David!

Thanks Thomas! Done.

Thanks Graham for taking my feedback on board. I agree with Simon's request too.

Fine from my perspective. The original changes were due to some language-lawyering to fix an awkward testcase, and I can completely believe my lawyering was wrong :)

Address review comments.

Hi Daniel,

Hi Thomas,

Looks trivially good to me.

Hi Graham,

Thanks David - I missed your comments, sorry this is a late response.

This feels like it could cause a pretty serious regression. This essentially disables global merging with -fdata-sections, which I know at least one linker relies upon for code size.

Hi Daniel,

LGTM, sorry for the long latency.

Thanks for pushing the revert. Apologies!

Thanks for pushing the rollback Mitch!

Thanks Jinsong! I've committed this as rL372376.

Thanks! rL372320.

Hi Simon,

Ah great, thanks for the context, and I'm glad this patch helps you out!

Thanks Simon! The patch was uploaded with -U999, but this one's with -U9999 just in case :)

Thanks Krzysztof!

Thankyou Daniel! Looks great!

Thanks Simon!

I will be very surprised if this does not cause any regressions at all.

I can think of a rationale for the current behaviour. A lopsided if-triangle could indicate it might be better to emit a branch than speculate. Especially as this transform is context-free, input code that contains the same pattern many times can cause an explosion in code size and the transform was likely designed to be conservative.

Adding Simon and Krzysztof as the original authors of InfoByHwMode.

Hi Roman,

Hi Roman,

Thanks for the great feedback Jinsong! I've addressed your comments.

Hi Thomas,

No-asserts build fixed in rL370894. Apologies for the breakage, I was holding git wrong.