[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;
----------------
hoy wrote:
> 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.
@mceier Could you do a favor by giving your suggested fix a try? I just realized that my system doesn't have a 32-bit toolset. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86495