[PATCH] D82525: [FPEnv] Intrinsics for access to FP control modes

[Matt Arsenault via Phabricator via llvm-commits]

arsenm requested changes to this revision.
arsenm added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4858-4860
+    auto *PtrTy = PointerType::get(*DAG.getContext(), DL.getAllocaAddrSpace());
+    Type *IntTy = DL.getIntPtrType(PtrTy);
+    SDValue Mode = DAG.getConstant(-1LL, dl, MVT::getVT(IntTy));
----------------
Don't think the alloca address space is correct for this. I guess use the default? Also just use TLI.getPointerTy, you don't need to go through the IR type


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4845
+        Node->getOperand(0), dl, Mode, StackPtr,
+        MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), SPFI));
+    Results.push_back(
----------------
sepavloff wrote:
> arsenm wrote:
> > This should be getStack, not getFixedStack
> Could you please explain a bit, why it should be `getStack`? It is not used anywhere in this file. Function `PerformInsertVectorEltInMemory` uses `getFixedStack` for temporary space, what is the difference between it and these cases?
Nevermind this is correct. We have confusing overlapping terminology. MachineFrameInfo refers to fixed stack objects as frame indexes as known fixed offsets for incoming stack arguments but apparently MachineMemOperand uses getFixedStack for any FrameIndex reference and getStack for offsets from SP


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82525/new/

https://reviews.llvm.org/D82525