[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:
> hoy wrote:
> > 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!
> Yes. SelectionDAG.cpp compiled with that change, I'm still waiting for whole build to finish.
Thanks for the update! Please let me know once the whole build is fine and I'll push the fix in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86495