Page MenuHomePhabricator

[IR][Verifier] Targets can allow alloca outside alloca address space
Needs ReviewPublic

Authored by wingo on Thu, Apr 22, 4:12 AM.

Details

Reviewers
tlively
arsenm
Summary

The WebAssembly target has a class of values that can't be written to
the stack, but would like to allow function-local variables to be
modelled using the usual alloca approach. These allocations will be
lowered to off-stack automatic-duration storage locations during
instruction selection. For this strategy to work, we need to relax the
restriction on alloca IR instructions.

Diff Detail

Event Timeline

wingo created this revision.Thu, Apr 22, 4:12 AM
wingo requested review of this revision.Thu, Apr 22, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 22, 4:12 AM
wingo added a subscriber: arsenm.Thu, Apr 22, 4:18 AM

Cc arsenm, as he added the address space restriction for alloca when he added address space support for alloca in https://github.com/llvm/llvm-project/commit/3c1fc768ed6cf2b01463df036ae6ae3b1f0de632.

wingo added a comment.Thu, Apr 22, 7:01 AM

Pre-merge checks show that https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/AMDGPU/invalid-alloca.ll fails; will need to follow up in a similar way.

The code looks fine, but explicitly requesting a review from @arsenm because I don't know the history of this restriction or how this affects other targets.

@wingo, have you investigated what would happen if we tried to use the normal stack address space in IR, but extracted reftype stack slots into a separate stack in the WebAssembly backend, perhaps specifically in WebAssemblyFrameLowering::emit{Prologue,Epilogue}?

I did envision this being useful, however there are a few points of concern:

  1. This does remove the safety check for the common case of creating an alloca without checking the datalayout's default alloca address space
  2. How is this going to be represented in codegen? In SelectionDAG the pointer type is immediately discarded and frame indexes don't have a tracked address space

I posted https://reviews.llvm.org/D101140 just for context -- based on this patch, and @pmatos's work on https://reviews.llvm.org/D95425.

@wingo, have you investigated what would happen if we tried to use the normal stack address space in IR, but extracted reftype stack slots into a separate stack in the WebAssembly backend, perhaps specifically in WebAssemblyFrameLowering::emit{Prologue,Epilogue}?

Humm, good question. The issue as I understand it would be that letting reference-typed values slide all the way to frame lowering would be what to do with uses of the alloca at instruction-selection time. For i32 locals, isel lowersa (set i32:$dst (load frameindex:$idx)) to i32.load, but what would we do for externref values?

But if we took the same solution as in my downstream patch, maybe keeping the alloca in AS 0 might with, where the ElementType is our AS 1 externref representation. Though, I do wonder what risk we might be introducing with regards to optimizations / transformations thinking that the pointer has a size and can be written to linear memory.

wingo added a comment.EditedFri, Apr 23, 12:18 AM

Thanks for your time, arsenm.

I did envision this being useful, however there are a few points of concern:

  1. This does remove the safety check for the common case of creating an alloca without checking the datalayout's default alloca address space

Yeah I can see why you might want this. If we want to keep the footgun-protection, one way would be to allow a target to explicitly opt out of the alloca address-space check. Something like:

target datalayout = "A1*"

to indicate that this target uses AS 1 by default for alloca, but that it can use others. Then we'd adapt the Verifier check to see if this datalayout allows alloca in other address spaces. WDYT?

  1. How is this going to be represented in codegen? In SelectionDAG the pointer type is immediately discarded and frame indexes don't have a tracked address space

We override TargetLowering::getPointerTy(DataLayout&, unsigned AS) to return a different MVT for these address spaces. We then have custom lowering for ISD::LOAD / ISD::STORE for those special MVTs.

Just for context, on the wasm target, each function has two stack frames: one managed by the webassembly run-time, populated by named locals, and not addressable; and one in memory managed as on conventional targets, sharing an address space with data and heap. Clang produces IR where locals start life as alloca's as usual, and as usual the optimizer lifts most of them to SSA values in SROA. SSA values end up lowering to the "named" part of the stack. Those locals still left as allocas are put on linear memory. This patch is motivated by ongoing work to add support for a new kind of value that can't be written to linear memory. We are currently representing these values as opaque pointers in non-default address spaces, and have some special lowering hooks for these. We also use some special address spaces to denote restricted pointers to these values.

wingo updated this revision to Diff 339939.Fri, Apr 23, 1:47 AM

Instead of globally relaxing the restriction on alloca address spaces,
allow targets to choose whether to impose the restriction via the
datalayout.

It hadn't occurred to me to add a field to the datalayout purely for the verifier. That seems a bit strange, since it's not exactly a target property.

What is the plan to use this in codegen?

llvm/docs/LangRef.rst
9566

c/designed/designated/?

Just for context, on the wasm target, each function has two stack frames: one managed by the webassembly run-time, populated by named locals, and not addressable; and one in memory managed as on conventional targets, sharing an address space with data and heap. Clang produces IR where locals start life as alloca's as usual, and as usual the optimizer lifts most of them to SSA values in SROA. SSA values end up lowering to the "named" part of the stack. Those locals still left as allocas are put on linear memory. This patch is motivated by ongoing work to add support for a new kind of value that can't be written to linear memory. We are currently representing these values as opaque pointers in non-default address spaces, and have some special lowering hooks for these. We also use some special address spaces to denote restricted pointers to these values.

But are these still tracked in the MachineFrameInfo like a regular frame index?

It hadn't occurred to me to add a field to the datalayout purely for the verifier. That seems a bit strange, since it's not exactly a target property.

Yeah understood. If we want some targets to have the check and some to not have the check, do you know of a better place to express this? I thought the same place that the target's alloca address space is defined might not be unreasonable, but I am new here :)

Just for context, on the wasm target, each function has two stack frames: one managed by the webassembly run-time, populated by named locals, and not addressable; and one in memory managed as on conventional targets, sharing an address space with data and heap. Clang produces IR where locals start life as alloca's as usual, and as usual the optimizer lifts most of them to SSA values in SROA. SSA values end up lowering to the "named" part of the stack. Those locals still left as allocas are put on linear memory. This patch is motivated by ongoing work to add support for a new kind of value that can't be written to linear memory. We are currently representing these values as opaque pointers in non-default address spaces, and have some special lowering hooks for these. We also use some special address spaces to denote restricted pointers to these values.

But are these still tracked in the MachineFrameInfo like a regular frame index?

They reach the MachineFrameInfo, but only ephemerally. At the beginning of isel, the static alloca's get collected and the MachineFrameInfo populated with abstract objects and frame indexes. Then when isel sees a load / store to these objects, the first load/store of the alloca causes it to be lifted out from the MachineFrameInfo and instead allocates it on the target-specific stack (as a WebAssembly local), and the object in the MachineFrameInfo is marked dead. Loads and stores to these objects lower to accesses to the parallel stack instead of to objects in MachineFrameInfo.

Obviously this doesn't allow for by-reference access to these allocations. This is OK for the WebAssembly target because these opaque types have source-level restrictions similar to the ones in place for ARM's SVE values -- see the ARM C Language Extensions (ACLE), §3.2.1 https://developer.arm.com/documentation/100987/0000.

wingo retitled this revision from [IR][Verifier] Allow alloca outside alloca address space to [IR][Verifier] Targets can allow alloca outside alloca address space.Tue, Apr 27, 1:39 AM
wingo marked an inline comment as done.Tue, Apr 27, 1:42 AM
wingo updated this revision to Diff 340756.Tue, Apr 27, 1:42 AM
  • Fix typo in LangRef