Cbieneman/out param draft assisted

agent_prompt.md

@@ -0,0 +1,20 @@
+# Initial Guidelines
+
+Please make sure that your changes are appropriately tested with unit tests
+covering each phase of translation in the compiler, and that your changes
+conform to the [LVLM Coding Standards](docs/CodingStandards.rst).
+
+Verify your changes by building and testing using the
+cmake/caches/PredefinedParams.cmake cache file with CMake's -C option and
+building the check-all target.
+
+Break your changes into small code changes with each change committed
+spearately. Record your thought process into a file named "agent_thoughts.md" at
+the root of the repository and commit it in its own commit when you're done.
+
+# Request
+
+I'm now seeing just a few test failures locally. Please fix the remaining test
+failures on the branch.
+
+See test_output.txt for details of my local failures.

include/dxc/Test/WEXAdapter.h

@@ -172,18 +172,20 @@ HRESULT TryGetValue(const wchar_t *param, Common::String &retStr);
 namespace Logging {
 namespace Log {
 inline void StartGroup(const wchar_t *name) {
-  wprintf(L"BEGIN TEST(S): <%ls>\n", name);
+  fprintf(stderr, "BEGIN TEST(S): <%ls>\n", name);
+  fflush(stderr);
 }
 inline void EndGroup(const wchar_t *name) {
-  wprintf(L"END TEST(S): <%ls>\n", name);
+  fprintf(stderr, "END TEST(S): <%ls>\n", name);
+  fflush(stderr);
 }
 inline void Comment(const wchar_t *msg) {
-  fputws(msg, stdout);
-  fputwc(L'\n', stdout);
+  fprintf(stdout, "%ls\n", msg);
+  fflush(stdout);
 }
 inline void Error(const wchar_t *msg) {
-  fputws(msg, stderr);
-  fputwc(L'\n', stderr);
+  fprintf(stderr, "%ls\n", msg);
+  fflush(stderr);
   ADD_FAILURE();
 }
 } // namespace Log

lib/HLSL/HLMatrixLowerPass.cpp

@@ -1667,6 +1667,38 @@ void HLMatrixLowerPass::lowerHLMatSubscript(
   while (GEPOperator *GEP = dyn_cast<GEPOperator>(RootPtr))
     RootPtr = GEP->getPointerOperand();
 
+  // Handle the case where the matrix pointer is an addrspacecast of an
+  // lvalue rooted at a global variable or alloca (e.g., a matrix field of a
+  // groupshared struct). CodeGen emits this addrspacecast whenever the
+  // matrix subscript intrinsic — which uses generic-address-space matrix
+  // pointers in its signature — is invoked on an lvalue in a non-zero
+  // address space. By the time this pass runs, the addrspacecast source may
+  // already be the lowered storage type (array of scalars), or still be a
+  // matrix-typed pointer that we need to bitcast to its lowered equivalent.
+  // In either case, tryGetLoweredPtrOperand bails on this pattern because
+  // the root is a global variable rather than an Argument or Alloca, so the
+  // HL subscript call would otherwise leak past matrix lowering.
+  if (LoweredPtr == nullptr) {
+    if (auto *ASC = dyn_cast<AddrSpaceCastInst>(MatPtr)) {
+      Value *SrcPtr = ASC->getOperand(0);
+      Value *SrcRoot = SrcPtr;
+      while (auto *GEP = dyn_cast<GEPOperator>(SrcRoot))
+        SrcRoot = GEP->getPointerOperand();
+      if (isa<GlobalVariable>(SrcRoot) || isa<AllocaInst>(SrcRoot)) {
+        Type *SrcElemTy = SrcPtr->getType()->getPointerElementType();
+        if (HLMatrixType::isa(SrcElemTy)) {
+          LoweredPtr = CallBuilder.CreateBitCast(
+              SrcPtr, HLMatrixType::getLoweredType(SrcPtr->getType()));
+        } else if (SrcElemTy->isArrayTy() || SrcElemTy->isVectorTy()) {
+          // Already lowered storage; use it directly.
+          LoweredPtr = SrcPtr;
+        }
+        if (LoweredPtr != nullptr)
+          RootPtr = SrcRoot;
+      }
+    }
+  }
+
   if (LoweredPtr == nullptr) {
     if (!isa<Argument>(RootPtr))
       return;

lib/HLSL/HLOperationLower.cpp

@@ -4488,6 +4488,50 @@ Value *TranslateBufLoad(ResLoadHelper &helper, HLResource::Kind RK,
   return FirstLd;
 }
 
+// For pointer-returning buffer loads with an HLSLOutArgExpr status argument,
+// the writeback sequence (load from temp alloca + store to actual dest) is
+// emitted immediately after the HL call. After SROA expands the result pointer
+// into GEP+load accesses and may optimize away the intermediate store,
+// UpdateStatus writes to the temp alloca AFTER the existing load of that alloca.
+// Fix this by moving the existing load instruction to after all UpdateStatus
+// stores so that mem2reg correctly propagates the checkAccessFullyMapped result.
+static void FixStatusLoadOrdering(Value *statusAlloca) {
+  AllocaInst *AI = dyn_cast<AllocaInst>(statusAlloca);
+  if (!AI)
+    return;
+  // Find the pre-existing load from the alloca (from the HLSLOutArgExpr writeback).
+  LoadInst *statusLoad = nullptr;
+  for (User *U : AI->users()) {
+    if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
+      statusLoad = LI;
+      break;
+    }
+  }
+  if (!statusLoad)
+    return;
+  // Find the last store to the alloca inserted by UpdateStatus.
+  BasicBlock *BB = statusLoad->getParent();
+  StoreInst *lastStore = nullptr;
+  for (Instruction &I : *BB) {
+    if (StoreInst *SI = dyn_cast<StoreInst>(&I)) {
+      if (SI->getPointerOperand() == AI)
+        lastStore = SI;
+    }
+  }
+  if (!lastStore)
+    return;
+  // Move the load to just after the last UpdateStatus store so that
+  // mem2reg propagates the correct checkAccessFullyMapped result.
+  // Check IR ordering: iterate the block to see if statusLoad comes before lastStore.
+  bool loadBeforeStore = false;
+  for (Instruction &I : *BB) {
+    if (&I == statusLoad) { loadBeforeStore = true; break; }
+    if (&I == lastStore) break;
+  }
+  if (loadBeforeStore)
+    statusLoad->moveBefore(lastStore->getNextNode());
+}
+
 Value *TranslateResourceLoad(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
                              HLOperationLowerHelper &helper,
                              HLObjectOperationLowerHelper *pObjHelper,
@@ -4509,6 +4553,11 @@ Value *TranslateResourceLoad(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
              "Textures should not be treated as structured buffers.");
     TranslateStructBufSubscript(cast<CallInst>(ldHelper.retVal), handle,
                                 ldHelper.status, hlslOP, RK, DL);
+    // After TranslateStructBufSubscript inserts UpdateStatus stores, move the
+    // pre-existing status load to after those stores so mem2reg propagates the
+    // checkAccessFullyMapped result rather than an uninitialized value.
+    if (ldHelper.status)
+      FixStatusLoadOrdering(ldHelper.status);
   } else {
     Ld = TranslateBufLoad(ldHelper, RK, Builder, hlslOP, DL);
     dxilutil::MigrateDebugValue(CI, Ld);

new_tests/array-tmp.hlsl

@@ -0,0 +1,7 @@
+void fn(float x[2]) { }
+
+float main(float val: A) : B {
+  float Arr[2] = {0, 0};
+  fn(Arr);
+  return Arr[0];
+}

new_tests/copy-vec.hlsl

@@ -0,0 +1,14 @@
+struct Agg {
+  float3 f3;
+};
+
+void get(out Agg agg);
+
+static Agg s_agg;
+
+export
+float3 main() {
+  get(s_agg);
+  return s_agg.f3;
+}
+

new_tests/gnarly-float-array.hlsl

@@ -0,0 +1,9 @@
+typedef int ai32[1];
+typedef float af32[1];
+void inc(inout float x) { x *= -1; }
+int main() : OUT
+{
+    ai32 x = { 42 };
+    inc(((af32)x)[0]);
+    return x[0];
+}

new_tests/implicit_truncation.hlsl

@@ -0,0 +1,16 @@
+struct Color {
+  uint16_t r;
+  uint16_t g;
+  uint16_t b;
+};
+
+RWStructuredBuffer<uint> buf : r0;
+
+[numthreads(4, 8, 16)]
+void main() {
+  Color s;
+  s.r = 4;
+  s.g = 5;
+  s.b = 6;
+  uint64_t value = (uint)s;
+}

new_tests/inout_lvalue.hlsl

@@ -0,0 +1,3 @@
+export void fn(inout float3 a, float3 b) {
+  a += b;
+}

new_tests/longlong.hlsl

@@ -0,0 +1,4 @@
+// dxc /Tvs_6_0 -spirv longlong.hlsl
+uint main() : A {
+   return vk::ReadClock(vk::SubgroupScope);
+}

new_tests/simple-inout.hlsl

@@ -0,0 +1,9 @@
+void fn(inout float x, inout int y) {
+  y = 2;
+  x = 1;
+}
+
+float main(float val: A) : B {
+  fn(val, val);
+  return val;
+}