summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTheresa Foley <10618364+tangent-vector@users.noreply.github.com>2025-10-14 18:00:47 -0700
committerGitHub <noreply@github.com>2025-10-15 01:00:47 +0000
commit907410f6a52cf4e7538870ebf5aeb88858f97973 (patch)
tree2c053154c23d828f5220a127f92ea2c98f706ce7
parent05cae938cbe1ac8807f4913044d9bee46dc3a0e1 (diff)
Clean up Slang IR representation of undefined values (#8708)
Prior to this change, the Slang IR used a single opcode (`kIROp_Undefined`) to encode all cases of undefined values. The particular motivation for this change was a need to distinguish those undefined values that represent a load from an uninitialized memory location versus other sorts of undefined values. If transforming a variable into SSA form results in `undefined` values in cases where the a `load` was executed without a prior `store`, that represents an error on the programmer's part, and should be diagnosed. However, other cases of undefined values can arise during program transformation and optimization, and should not typically result in diagnostics being emitted. While it was not the original motivation for this change, it is also worth noting that the LLVM project has transitioned from initially using only a single `undef` instruction to having a more nuanced model, and the same factors that motivated their shift also apply to the Slang IR. Counter-intuitively, the semantics of undefined values actually need to be carefully defined. Concretely, this change splits the pre-existing `undefined` opcode into two sub-cases: - `kIROp_LoadFromUninitializedMemory`, to represent the case of loading from a memory location (such as a local variable) that has not been initialized. - `kIROp_Poison`, corresponding to the LLVM `poison` value. Our poison instruction is intended to have semantics comparable to LLVM's equivalent. Conceptually, any operation that is invoked with a poison value as input will (with a few exceptions) produce a poison value as output. One can think of the behavior of `poison` as similar to how not-a-number values propagate in floating-point computations: by default they "infect" the result of any computation they are involved in. This semantic choice helps to ensure that many optimizations end up being correct in the presence of undefined values, even if they did not specifically account for them. The `kIROp_LoadFromUninitializedMemory` case is comparable to the combination of `freeze` and `undef` in LLVM. An LLVM `undef` value has semantics that allow *each* use of that value to be replaced with a *different* arbitrary value; these semantics cause many optimizations to only be correct in the absence of undefined values. An LLVM `freeze` instruction can take an undefined value as input, and produces a single value that is still arbitrary, but must be consistent across all uses. The latter semantics are what we want, since a given `load` from an uninitialized memory location will yield an arbitrary-but-fixed value. Note that we intentionally do not have a direct analogue to LLVM's `undef` instruction, because of the way that `undef` causes so many complications when trying to write optimizations. We also do not add a `kIROp_Freeze` instruction in this change, but that is simply because we currently have no need for it. Existing code that was creating `IRUndefined` values has been updated to create either `IRPoison` or `IRLoadFromUninitializedMemory` values, as appropriate to the use case. Code that was checking for the `kIROp_Undefined` opcode has been updated to either check for both of the new opcodes (in the case of `switch` statements), or to use `as<IRUndefined>` to perform a dynamic cast to the common base type of the two new instructions. Note that this change does not alter the way that instructions representing undefined values are typically emitted as ordinary instructions in the block that produces an undefined value. While emitting `IRLoadFromUninitializedMemory` as an ordinary instruction is exactly what we want, the `IRPoison` case would actually be better represented in Slang IR as a "hoistable" instruction, so that there would only be a singular `poison` value of each type. Changing `IRPoison` to be hoistable would be a good follow-up change, but might run into more challenges depending on what assumptions (if any) the codebase is making about where undefined values get emitted. --------- Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
-rw-r--r--source/slang/core.meta.slang2
-rw-r--r--source/slang/slang-emit-c-like.cpp8
-rw-r--r--source/slang/slang-emit-spirv.cpp3
-rw-r--r--source/slang/slang-emit-vm.cpp6
-rw-r--r--source/slang/slang-ir-autodiff-fwd.cpp5
-rw-r--r--source/slang/slang-ir-autodiff-primal-hoist.cpp3
-rw-r--r--source/slang/slang-ir-dce.cpp26
-rw-r--r--source/slang/slang-ir-eliminate-phis.cpp2
-rw-r--r--source/slang/slang-ir-glsl-legalize.cpp2
-rw-r--r--source/slang/slang-ir-insts-stable-names.lua3
-rw-r--r--source/slang/slang-ir-insts.h14
-rw-r--r--source/slang/slang-ir-insts.lua42
-rw-r--r--source/slang/slang-ir-legalize-empty-array.cpp55
-rw-r--r--source/slang/slang-ir-legalize-types.cpp27
-rw-r--r--source/slang/slang-ir-peephole.cpp37
-rw-r--r--source/slang/slang-ir-specialize.cpp2
-rw-r--r--source/slang/slang-ir-ssa.cpp12
-rw-r--r--source/slang/slang-ir-use-uninitialized-values.cpp2
-rw-r--r--source/slang/slang-ir-util.cpp6
-rw-r--r--source/slang/slang-ir-util.h5
-rw-r--r--source/slang/slang-ir.cpp18
-rw-r--r--source/slang/slang-lower-to-ir.cpp2
22 files changed, 207 insertions, 75 deletions
diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 9b55dc35a..ddee52b6b 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -3513,7 +3513,7 @@ __prefix T operator ~(T v0);
// IR level type traits.
__generic<T>
-__intrinsic_op($(kIROp_Undefined))
+__intrinsic_op($(kIROp_Poison)) // Note: attempting to actually *use* the result of `__declVal()` will always be invalid.
T __declVal();
__generic<T>
diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index d69485cce..11ea2e989 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -1679,7 +1679,7 @@ bool CLikeSourceEmitter::shouldFoldInstIntoUseSites(IRInst* inst)
// for GLSL), so we check this only after all those special cases are
// considered.
//
- if (inst->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(inst))
return false;
// Okay, at this point we know our instruction must have a single use.
@@ -2388,7 +2388,8 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
case kIROp_RTTIPointerType:
break;
- case kIROp_Undefined:
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
case kIROp_DefaultConstruct:
m_writer->emit(getName(inst));
break;
@@ -3245,7 +3246,8 @@ void CLikeSourceEmitter::_emitInst(IRInst* inst)
case kIROp_LiveRangeEnd:
emitLiveness(inst);
break;
- case kIROp_Undefined:
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
case kIROp_DefaultConstruct:
{
auto type = inst->getDataType();
diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp
index 8bcd1429f..d5697117a 100644
--- a/source/slang/slang-emit-spirv.cpp
+++ b/source/slang/slang-emit-spirv.cpp
@@ -4512,7 +4512,8 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
case kIROp_GetStringHash:
result = emitGetStringHash(inst);
break;
- case kIROp_Undefined:
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
result = emitOpUndef(parent, inst, inst->getDataType());
break;
case kIROp_SPIRVAsm:
diff --git a/source/slang/slang-emit-vm.cpp b/source/slang/slang-emit-vm.cpp
index 80d3762aa..71439ba0b 100644
--- a/source/slang/slang-emit-vm.cpp
+++ b/source/slang/slang-emit-vm.cpp
@@ -492,8 +492,12 @@ public:
{
switch (inst->getOp())
{
- case kIROp_Undefined:
+ case kIROp_Poison:
+ case kIROp_LoadFromUninitializedMemory:
{
+ // We basically handle an undefined value by allocating a
+ // temporary and then not initializing it.
+ //
ensureWorkingsetMemory(funcBuilder, inst);
}
break;
diff --git a/source/slang/slang-ir-autodiff-fwd.cpp b/source/slang/slang-ir-autodiff-fwd.cpp
index 946d5e7cb..5ca277672 100644
--- a/source/slang/slang-ir-autodiff-fwd.cpp
+++ b/source/slang/slang-ir-autodiff-fwd.cpp
@@ -752,7 +752,7 @@ InstPair ForwardDiffTranscriber::transcribeCall(IRBuilder* builder, IRCall* orig
SLANG_RELEASE_ASSERT(diffCalleeType->getParamCount() == origCall->getArgCount());
auto placeholderCall =
- builder->emitCallInst(nullptr, builder->emitUndefined(builder->getTypeKind()), 0, nullptr);
+ builder->emitCallInst(nullptr, builder->emitPoison(builder->getTypeKind()), 0, nullptr);
builder->setInsertBefore(placeholderCall);
IRBuilder argBuilder = *builder;
IRBuilder afterBuilder = argBuilder;
@@ -2143,7 +2143,8 @@ InstPair ForwardDiffTranscriber::transcribeInstImpl(IRBuilder* builder, IRInst*
case kIROp_DefaultConstruct:
return transcribeDefaultConstruct(builder, origInst);
- case kIROp_Undefined:
+ case kIROp_Poison:
+ case kIROp_LoadFromUninitializedMemory:
return transcribeUndefined(builder, origInst);
case kIROp_Reinterpret:
diff --git a/source/slang/slang-ir-autodiff-primal-hoist.cpp b/source/slang/slang-ir-autodiff-primal-hoist.cpp
index 5dea7f61b..eb85b9309 100644
--- a/source/slang/slang-ir-autodiff-primal-hoist.cpp
+++ b/source/slang/slang-ir-autodiff-primal-hoist.cpp
@@ -2667,7 +2667,8 @@ static bool shouldStoreInst(IRInst* inst)
case kIROp_ExtractExistentialValue:
case kIROp_ExtractExistentialType:
case kIROp_ExtractExistentialWitnessTable:
- case kIROp_Undefined:
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
case kIROp_GetSequentialID:
case kIROp_GetStringHash:
case kIROp_Specialize:
diff --git a/source/slang/slang-ir-dce.cpp b/source/slang/slang-ir-dce.cpp
index 0b89baf64..9578bf5e5 100644
--- a/source/slang/slang-ir-dce.cpp
+++ b/source/slang/slang-ir-dce.cpp
@@ -75,7 +75,7 @@ struct DeadCodeEliminationContext
}
}
- IRInst* getUndefInst()
+ IRInst* getUnitPoisonVal()
{
if (!undefInst)
{
@@ -84,7 +84,7 @@ struct DeadCodeEliminationContext
builder.setInsertBefore(firstChild);
else
builder.setInsertInto(module->getModuleInst());
- undefInst = Slang::getUndefInst(builder, module);
+ undefInst = Slang::getUnitPoisonVal(builder, module);
}
return undefInst;
}
@@ -109,12 +109,20 @@ struct DeadCodeEliminationContext
//
markInstAsLive(root);
- // Ensure there is a global undef inst that is always alive.
- // This undef inst will be used to fill in weak-referencing uses
- // whose used value is marked as dead and eliminated.
- // We always make sure this undef inst is available to prevent
- // infiniate oscilating loops.
- markInstAsLive(getUndefInst());
+ // We will use a single instruction representing an undefined
+ // value (using the `poison` instruction) as the replacement
+ // for any lingering uses of a value that is marked as dead
+ // (those uses logically represent "weak" references to the
+ // original value, since they didn't cause it to be kept alive).
+ //
+ // We need to make sure that this unique undefined value is
+ // treated as live, because if it is allowed to be removed then
+ // the compiler can get into an infinite oscillating loop where
+ // we determine that this instruction is not live and thus try
+ // to remove it and replace it with... a new instruction that
+ // is exactly like it.
+ //
+ markInstAsLive(getUnitPoisonVal());
// Marking the module as live should have
// seeded our work list, so we can now start
@@ -233,7 +241,7 @@ struct DeadCodeEliminationContext
//
if (inst->hasUses())
{
- inst->replaceUsesWith(getUndefInst());
+ inst->replaceUsesWith(getUnitPoisonVal());
}
if (inst->getOp() == kIROp_Param)
diff --git a/source/slang/slang-ir-eliminate-phis.cpp b/source/slang/slang-ir-eliminate-phis.cpp
index d5f1b9e43..47f5e17f9 100644
--- a/source/slang/slang-ir-eliminate-phis.cpp
+++ b/source/slang/slang-ir-eliminate-phis.cpp
@@ -1038,7 +1038,7 @@ struct PhiEliminationContext
// so that any logic that might have moved another parameter
// into a temporary will influence our result.
//
- if ((*srcArg.currentValPtr)->getOp() != kIROp_Undefined)
+ if (!as<IRUndefined>(*srcArg.currentValPtr))
{
// If we are trying to emit a store directly after a load from the same var,
// skip the store.
diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp
index 1511bbff2..1ecae6574 100644
--- a/source/slang/slang-ir-glsl-legalize.cpp
+++ b/source/slang/slang-ir-glsl-legalize.cpp
@@ -3529,7 +3529,7 @@ void legalizeEntryPointParameterForGLSL(
// operations like `TraceRay` are handled.
//
builder->setInsertBefore(func->getFirstBlock()->getFirstOrdinaryInst());
- auto undefinedVal = builder->emitUndefined(pp->getFullType());
+ auto undefinedVal = builder->emitPoison(pp->getFullType());
pp->replaceUsesWith(undefinedVal);
return;
diff --git a/source/slang/slang-ir-insts-stable-names.lua b/source/slang/slang-ir-insts-stable-names.lua
index a34dc346a..923e73ac4 100644
--- a/source/slang/slang-ir-insts-stable-names.lua
+++ b/source/slang/slang-ir-insts-stable-names.lua
@@ -156,7 +156,7 @@ return {
["Constant.blob_constant"] = 152,
["CapabilitySet.capabilityConjunction"] = 153,
["CapabilitySet.capabilityDisjunction"] = 154,
- ["undefined"] = 155,
+ ["Undefined.Poison"] = 155,
["defaultConstruct"] = 156,
["MakeDifferentialPairBase.MakeDiffPair"] = 157,
["MakeDifferentialPairBase.MakeDiffPairUserCode"] = 158,
@@ -680,4 +680,5 @@ return {
["SymbolAlias"] = 676,
["Decoration.InParamProxyVar"] = 677,
["Attr.MemoryScope"] = 678,
+ ["Undefined.LoadFromUninitializedMemory"] = 679,
}
diff --git a/source/slang/slang-ir-insts.h b/source/slang/slang-ir-insts.h
index 2255afc67..43adbcb64 100644
--- a/source/slang/slang-ir-insts.h
+++ b/source/slang/slang-ir-insts.h
@@ -2794,17 +2794,6 @@ struct IRRTTIObject : IRInst
FIDDLE(leafInst())
};
-// An instruction that yields an undefined value.
-//
-// Note that we make this an instruction rather than a value,
-// so that we will be able to identify a variable that is
-// used when undefined.
-FIDDLE()
-struct IRUndefined : IRInst
-{
- FIDDLE(leafInst())
-};
-
// Special inst for targets that support default initialization,
// like the braces '= {}' in C/HLSL
FIDDLE()
@@ -4277,7 +4266,8 @@ public:
IRInst* emitGpuForeach(List<IRInst*> args);
- IRUndefined* emitUndefined(IRType* type);
+ IRLoadFromUninitializedMemory* emitLoadFromUninitializedMemory(IRType* type);
+ IRPoison* emitPoison(IRType* type);
IRInst* emitReinterpret(IRInst* type, IRInst* value);
IRInst* emitOutImplicitCast(IRInst* type, IRInst* value);
diff --git a/source/slang/slang-ir-insts.lua b/source/slang/slang-ir-insts.lua
index e21fc86ae..ac72e718a 100644
--- a/source/slang/slang-ir-insts.lua
+++ b/source/slang/slang-ir-insts.lua
@@ -504,7 +504,47 @@ local insts = {
},
},
{ CapabilitySet = { hoistable = true, { capabilityConjunction = {} }, { capabilityDisjunction = {} } } },
- { undefined = {} },
+
+ -- Instructions that represent something with an undefined value.
+ { Undefined = {
+
+ -- A load from a memory location that is known to be uninitialized.
+ --
+ -- Primarily used so that the compiler front-end can diagnose an error on such cases.
+ --
+ -- A given `LoadFromUninitializedMemory` might evaluate to an arbitrary value of its type,
+ -- and an optimization pass may freely decide on a particular value to use and replace
+ -- all uses of the instruction with that value.
+ --
+ -- If there are multiple distinct `LoadFromUninitializedMemory` instructions, then they
+ -- might each yield a different value, even if they all reference the same memory
+ -- location.
+ --
+ -- Akin to `freeze(undefined)` in LLVM.
+ --
+ { LoadFromUninitializedMemory = {} },
+
+ -- An undefined value that is infectious.
+ --
+ -- Semantically, a poison value of some type T can be thought of as a
+ -- hypothetical out-of-band instance of type T, akin to a T-specific NaN
+ -- value (although a poison `float` is distinct from a `float` NaN value...).
+ -- The motivation for this interpretation is that it allows most optimizations
+ -- to ignore the possibility of poison/undefined values, while still being
+ -- semantically correct.
+ --
+ -- In most cases, an instruction that is executed with a poison value as one
+ -- of its operands yields a poison value as its result. The main exception
+ -- is instructions that only conditionally use an operand, such as `select`,
+ -- and block/function parameters (just because one branch passes a poison
+ -- argument for a parmeter, that doesn't mean the parameter would be poison
+ -- every time the block executes).
+ --
+ -- Corresponds to the LLVM `poison` instruction.
+ --
+ { Poison = {} },
+ }},
+
-- A `defaultConstruct` operation creates an initialized
-- value of the result type, and can only be used for types
-- where default construction is a meaningful thing to do.
diff --git a/source/slang/slang-ir-legalize-empty-array.cpp b/source/slang/slang-ir-legalize-empty-array.cpp
index 1e6798a2e..76e1874b7 100644
--- a/source/slang/slang-ir-legalize-empty-array.cpp
+++ b/source/slang/slang-ir-legalize-empty-array.cpp
@@ -50,9 +50,30 @@ struct EmptyArrayLoweringContext
return ptr && isEmptyArray(ptr->getValueType());
}
- // Visit each instruction and replace it with legalized instructiosn if necessary.
+ // Visit each instruction and replace it with legalized instructions if necessary.
void processInst(IRInst* inst)
{
+ //
+ // This function is handling several different replacements at once:
+ //
+ // * Instructions that would create an empty array value are changed
+ // to create a unit value (`void`) instead.
+ //
+ // * Instruction that would try to read an element from an empty array,
+ // or form a pointer to such an element, instead yield a `poison`
+ // (undefined) value. Note that they do not yield `LoadFromUninitializedMemory`
+ // because there is no guarantee that there would be anything in the
+ // memory corresponding to an index into an empty array.
+ //
+ // * Instructions that would try to read from an undefined pointer,
+ // buffer, image, etc. yield `poison`.
+ //
+ // * Instructions that would try to write to an undefined pointer,
+ // buffer, image, etc. are skipped (the behavior is undefined, so
+ // "do nothing" is a valid choice for the behavior, which has the
+ // nice side effect of potentially rendering other code redundant).
+ //
+
IRInst* replacement = nullptr;
IRBuilder builder(module);
@@ -65,64 +86,64 @@ struct EmptyArrayLoweringContext
[&](IRGetElement* getElement)
{
const auto base = getElement->getBase();
- return hasEmptyArrayType(base) ? builder.emitUndefined(getElement->getDataType())
+ return hasEmptyArrayType(base) ? builder.emitPoison(getElement->getDataType())
: nullptr;
},
[&](IRGetElementPtr* gep)
{
const auto base = gep->getBase();
return hasEmptyArrayPtrType(gep) || hasEmptyArrayPtrType(base) ||
- base->getOp() == kIROp_Undefined
- ? builder.emitUndefined(gep->getDataType())
+ as<IRUndefined>(base)
+ ? builder.emitPoison(gep->getDataType())
: nullptr;
},
[&](IRFieldAddress* gep)
{
const auto base = gep->getBase();
- return hasEmptyArrayPtrType(gep) || base->getOp() == kIROp_Undefined
- ? builder.emitUndefined(gep->getDataType())
+ return hasEmptyArrayPtrType(gep) || as<IRUndefined>(base)
+ ? builder.emitPoison(gep->getDataType())
: nullptr;
},
[&](IRLoad* load)
{
- return load->getOperand(0)->getOp() == kIROp_Undefined
- ? builder.emitUndefined(load->getDataType())
+ return as<IRUndefined>(load->getOperand(0))
+ ? builder.emitPoison(load->getDataType())
: nullptr;
},
[&](IRImageLoad* load)
{
- return load->getOperand(0)->getOp() == kIROp_Undefined
- ? builder.emitUndefined(load->getDataType())
+ return as<IRUndefined>(load->getOperand(0))
+ ? builder.emitPoison(load->getDataType())
: nullptr;
},
[&](IRStore* store)
{
- if (store->getPtr()->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(store->getPtr()))
store->removeAndDeallocate();
return nullptr;
},
[&](IRAtomicStore* store)
{
- if (store->getPtr()->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(store->getPtr()))
store->removeAndDeallocate();
return nullptr;
},
[&](IRImageStore* store)
{
- if (store->getImage()->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(store->getImage()))
store->removeAndDeallocate();
return nullptr;
},
[&](IRImageSubscript* subscript)
{
- return subscript->getImage()->getOp() == kIROp_Undefined
- ? builder.emitUndefined(subscript->getDataType())
+ return as<IRUndefined>(subscript->getImage())
+ ? builder.emitPoison(subscript->getDataType())
: nullptr;
},
[&](IRAtomicOperation* atomic)
{
- return atomic->getOperand(0)->getOp() == kIROp_Undefined
- ? builder.emitUndefined(atomic->getDataType())
+ return as<IRUndefined>(atomic->getOperand(0))
+ ? builder.emitPoison(atomic->getDataType())
: nullptr;
},
// The following should match any instruction which can construct a 0-sized array.
diff --git a/source/slang/slang-ir-legalize-types.cpp b/source/slang/slang-ir-legalize-types.cpp
index 021566e12..c6e8383c8 100644
--- a/source/slang/slang-ir-legalize-types.cpp
+++ b/source/slang/slang-ir-legalize-types.cpp
@@ -1952,6 +1952,18 @@ static LegalVal legalizeDefaultConstruct(IRTypeLegalizationContext* context, Leg
static LegalVal legalizeUndefined(IRTypeLegalizationContext* context, IRInst* inst)
{
+ // HACK: We check here if an undefined value we are trying to legalize is
+ // of an opaque type (such as a texture, buffer, or sampler), and issue
+ // a diagnostic (since there is really no way to make such code compile
+ // correctly for may of our downstream compilers).
+ //
+ // TODO(tfoley): This is emphatically *not* the right place for a diagnostic
+ // like this to be issued. We need rigorous checking for use of undefined
+ // values in the front-end. Once the front-end's checking can be trusted,
+ // the back-end should be able to remove any code that appears to use
+ // an uninitialized value of opaque type, since such code would have undefined
+ // behavior anyway.
+ //
IRType* opaqueType = nullptr;
if (isOpaqueType(inst->getFullType(), &opaqueType))
{
@@ -1962,6 +1974,18 @@ static LegalVal legalizeUndefined(IRTypeLegalizationContext* context, IRInst* in
context->m_sink->diagnose(loc, Diagnostics::useOfUninitializedOpaqueHandle, opaqueType);
}
+
+ // It is not ideal, but this pass legalizes an undefined value to... nothing.
+ //
+ // As a result, in any context that tries to consume a `LegalVal` based on its type,
+ // we need to be prepared not only for a value that matches the structure of that type,
+ // but also the possibility of an empty value like this.
+ //
+ // TODO(tfoley): We should *either* have a distinct case of `LegalVal` to represent
+ // undefined-ness, or this code should follow the same flow as the other cases by
+ // recursively decomposing the structure of the instruction's type and building up
+ // a `LegalVal` that just happens to have undefined values at its leaves.
+ //
return LegalVal();
}
@@ -2044,7 +2068,8 @@ static LegalVal legalizeInst(
case kIROp_Printf:
result = legalizePrintf(context, args);
break;
- case kIROp_Undefined:
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
return legalizeUndefined(context, inst);
case kIROp_GpuForeach:
// This case should only happen when compiling for a target that does not support
diff --git a/source/slang/slang-ir-peephole.cpp b/source/slang/slang-ir-peephole.cpp
index 8d6685212..79dfc1360 100644
--- a/source/slang/slang-ir-peephole.cpp
+++ b/source/slang/slang-ir-peephole.cpp
@@ -1041,7 +1041,7 @@ struct PeepholeContext : InstPassBase
continue;
SLANG_ASSERT(terminator->getArgCount() > paramIndex);
auto arg = terminator->getArg(paramIndex);
- if (arg->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(arg))
continue;
if (argValue == nullptr)
argValue = arg;
@@ -1268,14 +1268,18 @@ struct PeepholeContext : InstPassBase
}
case kIROp_Load:
{
- // Load from undef is undef.
- if (as<IRLoad>(inst)->getPtr()->getOp() == kIROp_Undefined)
+ // An attempt to load from an undefined pointer value
+ // is undefined behavior and the resulting value is poison
+ // (the value should typically contaminate instructions that
+ // use it, rendering them as poisonous).
+ //
+ if (as<IRUndefined>(as<IRLoad>(inst)->getPtr()))
{
IRBuilder builder(module);
IRBuilderSourceLocRAII srcLocRAII(&builder, inst->sourceLoc);
builder.setInsertBefore(inst);
- auto undef = builder.emitUndefined(inst->getDataType());
+ auto undef = builder.emitPoison(inst->getDataType());
inst->replaceUsesWith(undef);
maybeRemoveOldInst(inst);
changed = true;
@@ -1284,8 +1288,17 @@ struct PeepholeContext : InstPassBase
}
case kIROp_Store:
{
- // Store undef is no-op.
- if (as<IRStore>(inst)->getVal()->getOp() == kIROp_Undefined)
+ // An attempt to store to an undefined pointer value is
+ // undefined behavior (just like a load), so we can conveniently
+ // decide to implement that behavior as a no-op.
+ //
+ // TODO: While it is not the responsibility of a pass like this
+ // to diagnose errors (that is the front-end's job), it might
+ // be best to replace an invalid `store` like this with an
+ // instruction that represents a "panic" or similar exceptional
+ // situation.
+ //
+ if (as<IRUndefined>(as<IRStore>(inst)->getVal()))
{
maybeRemoveOldInst(inst);
changed = true;
@@ -1294,8 +1307,16 @@ struct PeepholeContext : InstPassBase
}
case kIROp_DebugValue:
{
- // Update debug value with undef is no-op.
- if (as<IRDebugValue>(inst)->getValue()->getOp() == kIROp_Undefined)
+ // Attempting to update the debug value of a variable with an
+ // undefined value will be treated as a no-op (meaning that the
+ // contents of the variable, as perceived by the user, will not
+ // change).
+ //
+ // TODO: We should probably validate that this is a reasonable
+ // behavior. In many cases a debugger user might like to have an
+ // indication of when the contents of their variable are undefined.
+ //
+ if (as<IRUndefined>(as<IRDebugValue>(inst)->getValue()))
{
maybeRemoveOldInst(inst);
changed = true;
diff --git a/source/slang/slang-ir-specialize.cpp b/source/slang/slang-ir-specialize.cpp
index 0f8672531..7ee2d4095 100644
--- a/source/slang/slang-ir-specialize.cpp
+++ b/source/slang/slang-ir-specialize.cpp
@@ -982,7 +982,7 @@ struct SpecializationContext
shouldSkip = true;
break;
}
- if (item->getOperand(i)->getOp() == kIROp_Undefined)
+ if (as<IRUndefined>(item->getOperand(i)))
{
shouldSkip = true;
break;
diff --git a/source/slang/slang-ir-ssa.cpp b/source/slang/slang-ir-ssa.cpp
index 789ca4796..dda72c813 100644
--- a/source/slang/slang-ir-ssa.cpp
+++ b/source/slang/slang-ir-ssa.cpp
@@ -690,10 +690,11 @@ IRInst* readVarRec(ConstructSSAContext* context, SSABlockInfo* blockInfo, IRVar*
// We would only reach this function (`readVarRec`) if
// a local lookup in the block had already failed, so
- // at this point we are dealing with an undefined value.
+ // at this point we are dealing with a read from a variable
+ // that has not yet been initialized.
auto type = var->getDataType()->getValueType();
- val = blockInfo->builder.emitUndefined(type);
+ val = blockInfo->builder.emitLoadFromUninitializedMemory(type);
cloneRelevantDecorations(var, val);
}
else if (!multiplePreds)
@@ -775,8 +776,13 @@ IRInst* readVar(ConstructSSAContext* context, SSABlockInfo* blockInfo, IRVar* va
// searching, because its value cannot have been
// established in a predecessor block.
//
+ // We already tried and failed to find a value set
+ // into the variable in this block, so we are left
+ // to conclude that the variable is uninitialized
+ // at this point.
+ //
auto type = var->getDataType()->getValueType();
- val = blockInfo->builder.emitUndefined(type);
+ val = blockInfo->builder.emitLoadFromUninitializedMemory(type);
cloneRelevantDecorations(var, val);
writeVar(context, blockInfo, var, val);
return val;
diff --git a/source/slang/slang-ir-use-uninitialized-values.cpp b/source/slang/slang-ir-use-uninitialized-values.cpp
index 200f29d1b..368f366c3 100644
--- a/source/slang/slang-ir-use-uninitialized-values.cpp
+++ b/source/slang/slang-ir-use-uninitialized-values.cpp
@@ -35,7 +35,7 @@ static bool isUninitializedValue(IRInst* inst)
// Also consider var since it does not
// automatically mean it will be initialized
// (at least not as the user may have intended)
- return (inst->m_op == kIROp_Undefined) || (inst->m_op == kIROp_Var);
+ return (as<IRUndefined>(inst) || (inst->m_op == kIROp_Var));
}
static bool isUnmodifying(IRFunc* func)
diff --git a/source/slang/slang-ir-util.cpp b/source/slang/slang-ir-util.cpp
index 5745ef85f..11eb4dfa0 100644
--- a/source/slang/slang-ir-util.cpp
+++ b/source/slang/slang-ir-util.cpp
@@ -1171,13 +1171,13 @@ bool canInstHaveSideEffectAtAddress(IRGlobalValueWithCode* func, IRInst* inst, I
return false;
}
-IRInst* getUndefInst(IRBuilder builder, IRModule* module)
+IRInst* getUnitPoisonVal(IRBuilder builder, IRModule* module)
{
IRInst* undefInst = nullptr;
for (auto inst : module->getModuleInst()->getChildren())
{
- if (inst->getOp() == kIROp_Undefined && inst->getDataType() &&
+ if (inst->getOp() == kIROp_Poison && inst->getDataType() &&
inst->getDataType()->getOp() == kIROp_VoidType)
{
undefInst = inst;
@@ -1188,7 +1188,7 @@ IRInst* getUndefInst(IRBuilder builder, IRModule* module)
{
auto voidType = builder.getVoidType();
builder.setInsertAfter(voidType);
- undefInst = builder.emitUndefined(voidType);
+ undefInst = builder.emitPoison(voidType);
}
return undefInst;
}
diff --git a/source/slang/slang-ir-util.h b/source/slang/slang-ir-util.h
index c12be51d6..0495bda0f 100644
--- a/source/slang/slang-ir-util.h
+++ b/source/slang/slang-ir-util.h
@@ -237,7 +237,10 @@ bool isPtrLikeOrHandleType(IRInst* type);
bool canInstHaveSideEffectAtAddress(IRGlobalValueWithCode* func, IRInst* inst, IRInst* addr);
-IRInst* getUndefInst(IRBuilder builder, IRModule* module);
+/// Get a unit-type (aka `void`) value using the `poison` instruction,
+/// which indicates an undefined (and potentially unstable) value.
+///
+IRInst* getUnitPoisonVal(IRBuilder builder, IRModule* module);
// The the equivalent op of (a op b) in (b op' a). For example, a > b is equivalent to b < a. So (<)
// ==> (>).
diff --git a/source/slang/slang-ir.cpp b/source/slang/slang-ir.cpp
index 8371d6ef5..061edd966 100644
--- a/source/slang/slang-ir.cpp
+++ b/source/slang/slang-ir.cpp
@@ -3384,13 +3384,18 @@ IRInst* IRBuilder::emitGetValueFromBoundInterface(IRType* type, IRInst* boundInt
return inst;
}
-
-IRUndefined* IRBuilder::emitUndefined(IRType* type)
+IRLoadFromUninitializedMemory* IRBuilder::emitLoadFromUninitializedMemory(IRType* type)
{
- auto inst = createInst<IRUndefined>(this, kIROp_Undefined, type);
-
+ auto inst =
+ createInst<IRLoadFromUninitializedMemory>(this, kIROp_LoadFromUninitializedMemory, type);
addInst(inst);
+ return inst;
+}
+IRPoison* IRBuilder::emitPoison(IRType* type)
+{
+ auto inst = createInst<IRPoison>(this, kIROp_Poison, type);
+ addInst(inst);
return inst;
}
@@ -8681,8 +8686,11 @@ bool IRInst::mightHaveSideEffects(SideEffectAnalysisOptions options)
case kIROp_LiveRangeStart:
case kIROp_LiveRangeEnd:
+ // Undefined values are treated as having no side effects.
+ case kIROp_LoadFromUninitializedMemory:
+ case kIROp_Poison:
+
case kIROp_Nop:
- case kIROp_Undefined:
case kIROp_DefaultConstruct:
case kIROp_Specialize:
case kIROp_LookupWitnessMethod:
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index 80aa6066a..2708c0192 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -2028,7 +2028,7 @@ struct ValLoweringVisitor : ValVisitor<ValLoweringVisitor, LoweredValInfo, Lower
operands.add(argVal);
});
- auto undefined = getBuilder()->emitUndefined(operands[1]->getFullType());
+ auto undefined = getBuilder()->emitPoison(operands[1]->getFullType());
return getBuilder()->getDifferentialPairUserCodeType(primalType, undefined);
}
else