[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Wei Mi via Phabricator via cfe-commits]

wmi added a comment.

I limit the bitfield separation in the last update to only happen at the beginning of a run so no bitfield combine will be blocked.

I think I need to explain more about why I change the direction and start to work on the problem in frontend. Keeping the information by generating widening type and letting llvm pass to do narrowing looks like a good solution to me originally. However, I saw in real cases that the narrowing approach in a late llvm stage has more difficulties than I originally thought. Some of them are solved but at the cost of code complexity, but others are more difficult.

- store forwarding issue: To extract legal integer width bitfields from a large integer generated by frontend, we need to split both stores and loads related with legal integer bitfields. If store is narrowed and load is not, the width of load may be smaller than the store and the target may have difficulty to do store forwarding and that fact will hit the performance.  Note, we found case that related load and store are in different funcs, so when deciding whether to narrow a store or not, it is possible that we have no idea that the related load is narrowed or not. If we cannot know all the related loads will be narrowed, the store is better not to be narrowed.

- After instcombine, some bitfield access information will be lost: The case we saw is: unsigned long bf1 : 16 unsigned long bf2 : 16 unsigned long bf3 : 16 unsigned long bf4 : 8

  bool cond = "bf3 == 0 && bf4 == -1";

  Before instcombine, bf3 and bf4 are extracted from an i64 separately. We can know bf3 is a 16 bits access and bf4 is a 8 bit access from the extracting code pattern. After instcombine, bf3 and bf4 are merged into a 24 bit access, the comparison above is changed to: extract 24 bit data from the i64 (%bf.load = wide load i64, %extract = and %bf.load, 0xffffff00000000) and compare %extract with 0xffff00000000. The information that there are two legal integer bitfield accesses is lost, and we won't do narrowing for the load here.

  Because we cannot split the load here, we trigger store forwarding issue.

That is why I am exploring to work on the bitfield access issue in multiple directions.


Repository:
  rL LLVM

https://reviews.llvm.org/D36562