[PATCH] D39028: Irreducible loop metadata for more accurate block frequency under PGO.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:1161
+      const BlockT *Block = getBlock(HeaderNode);
+      if (Optional<uint64_t> HeaderWeight = Block->getIrrLoopHeaderWeight()) {
+        DEBUG(dbgs() << getBlockName(HeaderNode)
----------------
may be 

 if ( !(... HeaderWeight = )) 
    continue;


================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:1166
+        NumHeadersWithWeight++;
+        if (HeaderWeight.getValue())
+          Dist.addLocal(HeaderNode, HeaderWeight.getValue());
----------------
Is this check needed? 


================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:1172
+    if (NumHeadersWithWeight != Loop.NumHeaders) {
+      // Non-PGO
+      BlockMass Remaining = BlockMass::getFull();
----------------
Can this share the same code with 'distributeIrrLoopHeaderMass'  by making weights evenly distributed?


================
Comment at: lib/IR/MDBuilder.cpp:203
+  SmallVector<Metadata *, 2> Vals(2);
+  Vals[0] = createString("header_weight");
+  Vals[1] = createConstant(ConstantInt::get(Type::getInt64Ty(Context), Weight));
----------------
loop_header_weight?


================
Comment at: test/Analysis/BlockFrequencyInfo/irreducible_pgo.ll:5
+; Function Attrs: noinline norecurse nounwind readnone uwtable
+define i32 @_Z11irreducibleii(i32 %iter_outer, i32 %iter_inner) local_unnamed_addr !prof !27 {
+entry:
----------------
Can this test case be minimized (eg. shrinking BB size ..) ?


https://reviews.llvm.org/D39028