[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Wei Mi via Phabricator via cfe-commits]

wmi added a comment.

I think it may be hard to fix the problem in backend. It will face the same issue of store-to-load forwarding if at some places the transformation happens but at some other places somehow it doesn't.

But I am not sure whether it is acceptable to add more finegrain bitfield access transformations in frontend. It is a tradeoff. From my experience trying your patch on x8664, I saw saving of some instructions because with the transformation we now use shorter immediate consts which can be folded into other instructions instead of loading them in separate moves. But the bit operations are not saved (I may be wrong), so I have no idea whether it is critical for performance (To provide some background, we introduced finegrain field access because the original problem introduced a serious performance issue in some critical libraries. But doing this kind of bitfield transformations in frontend can potentially harm some other applications. That is why it is off by default, and Hal had concern about adding more such transformations. ).  Do you have performance number on some benchmarks to justify its importance? That may help folks to make a better trade off here.


https://reviews.llvm.org/D39053