[PATCH] D76880: [AMDGPU] Emit entry function CFI

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:394
+                                        // should be defined elsewhere
+      static_cast<char>(unsigned(dwarf::DW_OP_LLVM_form_aspace_address))};
+  BuildCFI(MBB, I, DL,
----------------
Why two levels of casting in all of these places? I also really don't like the constructor form of casts


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1074-1085
+void SIFrameLowering::BuildCFI(MachineBasicBlock &MBB,
+                               MachineBasicBlock::iterator MBBI,
+                               const DebugLoc &DL,
+                               const MCCFIInstruction &CFIInst) const {
+  MachineFunction &MF = *MBB.getParent();
+  const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
+  const SIInstrInfo *TII = ST.getInstrInfo();
----------------
I don't think this function buys you much since all of these thing are already in scope at the use.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.h:78
+  /// Create a CFI index for CFIInst and build a MachineInstr around it.
+  void BuildCFI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+                const DebugLoc &DL, const MCCFIInstruction &CFIInst) const;
----------------
Star with lowercase 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76880