This is an archive of the discontinued LLVM Phabricator instance.

don't Constant Hoist 1 bit bitmasks in X86
Needs ReviewPublic

Authored by CSears on Feb 4 2015, 6:59 PM.
This revision needs review, but all specified reviewers are disabled or inactive.

Details

Summary

This patch fixes a bug/pessimization which I've tracked down for 1 bit bitmasks.

if (((xx) & (1ULL << (40))))

return 1;

if (!((yy) & (1ULL << (40))))

...

The second time Constant Hoisting sees the value (1<<40) it wraps it up with a bitcast.
That value then gets hoisted. However, the first (1<<40) is not bitcast and gets recognized
and substituted as a BT. The second doesn't get recognised because of the hoisting.

The result is some register allocation and unnecessary constant loading instructions.

There are maybe three 'solutions' to this problem, maybe more.

Starting with the second, in the middle of things, you could try pattern matching in
EmitTest() or LowerToBT(). I've tried this and it doesn't work since it needs to reach
outside of a Selection DAG. Doesn't work. Can't work.

Thirdly, it's been suggested to use a peephole pass and to look at
AArch64LoadStoreOptimizer.cpp. This also doesn't work for pretty much the
same reason. Moreover, this is after register allocation so even for the limited
situations where it can work, it leaves allocated but unutilized registers.
Doesn't work. In fact, I'd suggest the Arm backend adopt my approach.

So firstly, I think the best way to solve this problem is to avoid this problem
in the first place. Just don't hoist these values.

For the X86 backend, X86TTI::getIntImmCost() in X86TargetTransformInfo.cpp
is an overridden function. Just mark these 1 bit masks there as TCC_Free:

// Don't hoist 1 bit masks. They'll probably be used for BT, BTS, BTC.
if (Imm.isPowerOf2())       // this could be limited to bits 32-63
  return TCC_Free;

This works. Its only downside is when these values are being used twice
AND then not being combined into another instruction.

I've narrowed this to between llvm 3.4.1 and XCode llvm 3.5. It isn't present in 3.4.1
and it is present in XCode llvm 3.5.

Diff Detail

Event Timeline

CSears updated this revision to Diff 19376.Feb 4 2015, 6:59 PM
CSears retitled this revision from to don't Constant Hoist 1 bit bitmasks in X86.
CSears updated this object.
CSears edited the test plan for this revision. (Show Details)
CSears added a reviewer: joker-eph-DISABLED.
CSears added a subscriber: Unknown Object (MLST).