This is an archive of the discontinued LLVM Phabricator instance.

[WIP][Statepoint Lowering] Allow other than N byte sized types in deopt bundle
Needs ReviewPublic

Authored by yrouban on Apr 6 2021, 8:09 PM.

Details

Summary

There could be a need for deopt bundle operand's stack slots to be N byte aligned.
This patch introduces an option -promote-slot-bits-in-selection-dag that specifies minimum bit size for such slots.

Diff Detail

Event Timeline

yrouban created this revision.Apr 6 2021, 8:09 PM
yrouban requested review of this revision.Apr 6 2021, 8:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 8:09 PM

Any reason why do you specify alignment in bits instead of bytes?
Is there any (supported by LLVM) CPU architecture with bit-addressing of memory?
(That really look strange to me, given byte size is hardcoded to 8 bits)

reames added a comment.Apr 7 2021, 8:37 PM

Any reason why do you specify alignment in bits instead of bytes?
Is there any (supported by LLVM) CPU architecture with bit-addressing of memory?
(That really look strange to me, given byte size is hardcoded to 8 bits)

Alignment is specified in bytes in IR. LLVM does not (currently) support bit addressable targets.

yrouban planned changes to this revision.Apr 8 2021, 4:36 AM
yrouban added inline comments.
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
186

64 should be replaced with PromoteSlotBitsInSelectionDAG

reames added inline comments.Apr 8 2021, 8:08 AM
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
62

I don't think a command line argument is the right way to describe this. I strongly suspect that this number is a property of the calling convention of the call (or maybe a combination of the callers calling convention and the calls calling convention.) If so, I would suggest reframing it in that manner.

For calling conventions which don't explicitly override it to something wider, using the abi alignment (e.g. the same we'd use for argument passing) seems like a reasonable default. That should also give you way to write tests without upstreaming the custom calling convention motivating this.

179

Style: Please use early return.

411–417

A suggestion.

If you split the promote function into one which produced the promoted type, and one which did the actual promotion, I think you can simplify this code, make it easier to keep in sync, and probably simplify some of your asserts.

429

In particular, previous suggestion should greatly simplify this block of code.

yrouban updated this revision to Diff 336129.Apr 8 2021, 8:39 AM
yrouban retitled this revision from [Statepoint Lowering] Allow other than N byte sized types in deopt bundle to [WIP][Statepoint Lowering] Allow other than N byte sized types in deopt bundle.

Added 3 test cases with live-in deopt values that crash.
Removed one unrelated comment .
Fixed couple of places with hardcoded 64 const.
Marked the patch as work-in-progress.