[PATCH] D86495: [CSSPGO] MIR target-independent pseudo instruction for pseudo-probe intrinsic

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAGNodes.h:1752-1777
+/// This SDNode is used for PSEUDO_PROBE values, which are the function guid and
+/// the index of the basic block being probed. A pseudo probe serves as a place
+/// holder and will be removed at the end of compilation. It does not have any
+/// operand because we do not want the instruction selection to deal with any.
+class PseudoProbeSDNode : public SDNode {
+  friend class SelectionDAG;
+  uint64_t Guid;
----------------
mceier wrote:
> The size of this class (66 if I computed it correctly) exceeds the size of LargestSDNode (64 bytes) in llvm/include/llvm/CodeGen/SelectionDAGNodes.h causing static_assert to fail in Recycler.h when compiling SelectionDAG.cpp (at least) on 32-bit system:
> 
> 
> ```
> In file included from llvm/include/llvm/CodeGen/MachineFunction.h:35,
>                  from llvm/include/llvm/CodeGen/SelectionDAG.h:31,
>                  from llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:13:
> llvm/include/llvm/Support/Recycler.h: In instantiation of ‘SubClass* llvm::Recycler<T, Size, Align>::Allocate(AllocatorType&) [with SubClass = llvm::PseudoProbeSDNode; AllocatorType = llvm::BumpPtrAllocatorImpl<>; T = llvm::SDNode; unsigned int Size = 64; unsigned int Align = 4]’:
> llvm/include/llvm/Support/RecyclingAllocator.h:43:65:   required from ‘SubClass* llvm::RecyclingAllocator<AllocatorType, T, Size, Align>::Allocate() [with SubClass = llvm::PseudoProbeSDNode; AllocatorType = llvm::BumpPtrAllocatorImpl<>; T = llvm::SDNode; unsigned int Size = 64; unsigned int Align = 4]’
> llvm/include/llvm/CodeGen/SelectionDAG.h:380:57:   required from ‘SDNodeT* llvm::SelectionDAG::newSDNode(ArgTypes&& ...) [with SDNodeT = llvm::PseudoProbeSDNode; ArgTypes = {const unsigned int&, unsigned int, const llvm::DebugLoc&, const llvm::SDVTList&, long long unsigned int&, long long unsigned int&, unsigned int&}]’
> llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6906:72:   required from here
> llvm/include/llvm/Support/Recycler.h:86:36: error: static assertion failed: Recycler allocation size is less than object size!
>    86 |     static_assert(sizeof(SubClass) <= Size,
>       |                   ~~~~~~~~~~~~~~~~~^~~~~~~
> ```
> 
> Relevant bug: https://bugs.llvm.org/show_bug.cgi?id=48259
> 
> I'm guessing this code in llvm/include/llvm/CodeGen/SelectionDAGNodes.h:
> 
> ```
> using LargestSDNode = AlignedCharArrayUnion<AtomicSDNode, TargetIndexSDNode,
>                                             BlockAddressSDNode,
>                                             GlobalAddressSDNode>;
> ```
> 
> should be changed to:
> 
> ```
> using LargestSDNode = AlignedCharArrayUnion<AtomicSDNode, TargetIndexSDNode,
>                                             BlockAddressSDNode,
>                                             GlobalAddressSDNode,
>                                             PseudoProbeSDNode>;
> ```
Thanks for reporting the bug! I'm sorry for the breakage. Will send out a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86495