[Remarks] Provide more information about auto-init stores

[thegameg]

Summary

This adds support for analyzing the instruction with the !annotation
"auto-init" in order to generate a more user-friendly remark.

For now, support the store size, and whether it's atomic/volatile.

Example:

auto-init.c:4:7: remark: Store inserted by -ftrivial-auto-var-init.Store size: 4 bytes. [-Rpass-missed=annotation-remarks]
  int var;
      ^

[paquette]

llvm/lib/Transforms/Utils/AutoInitRemark.cpp

30

I think it may be worth handling the false case here as well. People may want to search for non-volatile and non-atomic stores in the remarks.

[thegameg]

llvm/lib/Transforms/Utils/AutoInitRemark.cpp

30

In my experience volatile/atomic auto-init stores are very uncommon. While I agree that it's convenient for scripts and such to have it anyway, I wonder if it doesn't pollute the remark message too much.

Maybe I can put it under setExtraArgs...

[thegameg]

Add StoreVolatile: false and StoreAtomic: false under ExtraArgs.

[thegameg]

llvm/lib/Transforms/Utils/AutoInitRemark.cpp

30

Yep, that worked, thanks!

[paquette]

LGTM

llvm/include/llvm/Transforms/Utils/AutoInitRemark.h

+//===- AutoInitRemark.h - Auto-init remark analysis -*- C++ -------------*-===//
+//
+//                      The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Provide more information about instructions with a "auto-init"
+// !annotation metadata.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_UTILS_AUTOINITREMARK_H
+#define LLVM_TRANSFORMS_UTILS_AUTOINITREMARK_H
+
+#include "llvm/ADT/StringRef.h"
+
+namespace llvm {
+
+class StoreInst;
+class Instruction;
+class OptimizationRemarkEmitter;
+class DataLayout;
+
+// FIXME: Once we get to more remarks like this one, we need to re-evaluate how
+// much of this logic should actually go into the remark emitter.
+struct AutoInitRemark {
+  OptimizationRemarkEmitter &ORE;
+  StringRef RemarkPass;
+  const DataLayout &DL;
+
+  AutoInitRemark(OptimizationRemarkEmitter &ORE, StringRef RemarkPass,
+                 const DataLayout &DL)
+      : ORE(ORE), RemarkPass(RemarkPass), DL(DL) {}
+
+  void inspectStore(StoreInst &SI);
+  void inspectUnknown(Instruction &I);
+};
+
+} // namespace llvm
+
+#endif

llvm/lib/Transforms/Scalar/AnnotationRemarks.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Scalar/AnnotationRemarks.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Utils/AutoInitRemark.h"
 
 using namespace llvm;
 using namespace llvm::ore;
 
 #define DEBUG_TYPE "annotation-remarks"
 #define REMARK_PASS DEBUG_TYPE
 
 static void tryEmitAutoInitRemark(ArrayRef<Instruction *> Instructions,
                                   OptimizationRemarkEmitter &ORE) {
   // For every auto-init annotation generate a separate remark.
   for (Instruction *I : Instructions) {
     if (!I->hasMetadata(LLVMContext::MD_annotation))
       continue;
     for (const MDOperand &Op :
          I->getMetadata(LLVMContext::MD_annotation)->operands()) {
       if (cast<MDString>(Op.get())->getString() != "auto-init")
         continue;
 
-      ORE.emit(
-          OptimizationRemarkMissed(REMARK_PASS, "AutoInitUnknownInstruction", I)
-          << "Initialization inserted by -ftrivial-auto-var-init.");
+      Function &F = *I->getParent()->getParent();
+      const DataLayout &DL = F.getParent()->getDataLayout();
+      AutoInitRemark Remark(ORE, REMARK_PASS, DL);
+      // For some of them, we can provide more information:
+
+      // For stores:
+      // * size
+      // * volatile / atomic
+      if (auto *SI = dyn_cast<StoreInst>(I)) {
+        Remark.inspectStore(*SI);
+        continue;
+      }
+
+      Remark.inspectUnknown(*I);
     }
   }
 }
 
 static void runImpl(Function &F) {
   if (!OptimizationRemarkEmitter::allowExtraAnalysis(F, REMARK_PASS))
     return;
 

llvm/lib/Transforms/Utils/AutoInitRemark.cpp

+//===-- AutoInitRemark.cpp - Auto-init remark analysis---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementation of the analysis for the "auto-init" remark.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/AutoInitRemark.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/IR/Instructions.h"
+
+using namespace llvm;
+using namespace llvm::ore;
+
+void AutoInitRemark::inspectStore(StoreInst &SI) {
+  bool Volatile = SI.isVolatile();
+  bool Atomic = SI.isAtomic();
+  int64_t Size = DL.getTypeStoreSize(SI.getOperand(0)->getType());
+
+  OptimizationRemarkMissed R(RemarkPass.data(), "AutoInitStore", &SI);
+  R << "Store inserted by -ftrivial-auto-var-init.\nStore size: "
+    << NV("StoreSize", Size) << " bytes.";
+  if (Volatile)
+    R << " Volatile: " << NV("StoreVolatile", true) << ".";
+  if (Atomic)
+    R << " Atomic: " << NV("StoreAtomic", true) << ".";
+  // Emit StoreVolatile: false and StoreAtomic: false under ExtraArgs. This
+  // won't show them in the remark message but will end up in the serialized
+  // remarks.
+  if (!Volatile || !Atomic)
+    R << setExtraArgs();
+  if (!Volatile)
+    R << " Volatile: " << NV("StoreVolatile", false) << ".";
+  if (!Atomic)
+    R << " Atomic: " << NV("StoreAtomic", false) << ".";
+
+  ORE.emit(R);
+}
+
+void AutoInitRemark::inspectUnknown(Instruction &I) {
+  ORE.emit(OptimizationRemarkMissed(RemarkPass.data(),
+                                    "AutoInitUnknownInstruction", &I)
+           << "Initialization inserted by -ftrivial-auto-var-init.");
+}

llvm/lib/Transforms/Utils/CMakeLists.txt

 add_llvm_component_library(LLVMTransformUtils
   AddDiscriminators.cpp
   AMDGPUEmitPrintf.cpp
   ASanStackFrameLayout.cpp
   AssumeBundleBuilder.cpp
+  AutoInitRemark.cpp
   BasicBlockUtils.cpp
   BreakCriticalEdges.cpp
   BuildLibCalls.cpp
   BypassSlowDivision.cpp
   CallPromotionUtils.cpp
   CallGraphUpdater.cpp
   CanonicalizeAliases.cpp
   CanonicalizeFreezeInLoops.cpp

llvm/test/Transforms/Util/trivial-auto-var-init-store.ll

 ; RUN: opt -annotation-remarks -o /dev/null -S -pass-remarks-output=%t.opt.yaml %s -pass-remarks-missed=annotation-remarks 2>&1 | FileCheck %s
 ; RUN: cat %t.opt.yaml | FileCheck -check-prefix=YAML %s
 
 ; Emit a remark that reports a store.
 define void @store(i32* %dst) {
-; CHECK: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK:      Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
 ; YAML-LABEL: --- !Missed
 ; YAML-NEXT: Pass:            annotation-remarks
-; YAML-NEXT: Name:            AutoInitUnknownInstruction
+; YAML-NEXT: Name:            AutoInitStore
 ; YAML-NEXT: DebugLoc:
 ; YAML-NEXT: Function:        store
 ; YAML-NEXT: Args:
-; YAML-NEXT:   - String:          Initialization inserted by -ftrivial-auto-var-init.
+; YAML-NEXT:   - String:          "Store inserted by -ftrivial-auto-var-init.\nStore size: "
+; YAML-NEXT:   - StoreSize:       '4'
+; YAML-NEXT:   - String:          ' bytes.'
+; YAML-NEXT:   - String:          ' Volatile: '
+; YAML-NEXT:   - StoreVolatile:       'false'
+; YAML-NEXT:   - String:          .
+; YAML-NEXT:   - String:          ' Atomic: '
+; YAML-NEXT:   - StoreAtomic:       'false'
+; YAML-NEXT:   - String:          .
 ; YAML-NEXT: ...
   store i32 0, i32* %dst, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a volatile store.
 define void @volatile_store(i32* %dst) {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes. Volatile: true.
 ; YAML-LABEL: --- !Missed
 ; YAML-NEXT: Pass:            annotation-remarks
-; YAML-NEXT: Name:            AutoInitUnknownInstruction
+; YAML-NEXT: Name:            AutoInitStore
 ; YAML-NEXT: DebugLoc:
 ; YAML-NEXT: Function:        volatile_store
 ; YAML-NEXT: Args:
-; YAML-NEXT:   - String:          Initialization inserted by -ftrivial-auto-var-init.
+; YAML-NEXT:   - String:          "Store inserted by -ftrivial-auto-var-init.\nStore size: "
+; YAML-NEXT:   - StoreSize:       '4'
+; YAML-NEXT:   - String:          ' bytes.'
+; YAML-NEXT:   - String:          ' Volatile: '
+; YAML-NEXT:   - StoreVolatile:       'true'
+; YAML-NEXT:   - String:          .
+; YAML-NEXT:   - String:          ' Atomic: '
+; YAML-NEXT:   - StoreAtomic:       'false'
+; YAML-NEXT:   - String:          .
 ; YAML-NEXT: ...
   store volatile i32 0, i32* %dst, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports an atomic store.
 define void @atomic_store(i32* %dst) {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes. Atomic: true.
 ; YAML-LABEL: --- !Missed
 ; YAML-NEXT: Pass:            annotation-remarks
-; YAML-NEXT: Name:            AutoInitUnknownInstruction
+; YAML-NEXT: Name:            AutoInitStore
 ; YAML-NEXT: DebugLoc:
 ; YAML-NEXT: Function:        atomic_store
 ; YAML-NEXT: Args:
-; YAML-NEXT:   - String:          Initialization inserted by -ftrivial-auto-var-init.
+; YAML-NEXT:   - String:          "Store inserted by -ftrivial-auto-var-init.\nStore size: "
+; YAML-NEXT:   - StoreSize:       '4'
+; YAML-NEXT:   - String:          ' bytes.'
+; YAML-NEXT:   - String:          ' Atomic: '
+; YAML-NEXT:   - StoreAtomic:       'true'
+; YAML-NEXT:   - String:          .
+; YAML-NEXT:   - String:          ' Volatile: '
+; YAML-NEXT:   - StoreVolatile:       'false'
+; YAML-NEXT:   - String:          .
 ; YAML-NEXT: ...
   store atomic i32 0, i32* %dst unordered, align 4, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca.
 define void @store_alloca() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
 ; YAML-LABEL: --- !Missed
 ; YAML-NEXT: Pass:            annotation-remarks
-; YAML-NEXT: Name:            AutoInitUnknownInstruction
+; YAML-NEXT: Name:            AutoInitStore
 ; YAML-NEXT: DebugLoc:
 ; YAML-NEXT: Function:        store_alloca
 ; YAML-NEXT: Args:
-; YAML-NEXT:   - String:          Initialization inserted by -ftrivial-auto-var-init.
+; YAML-NEXT:   - String:          "Store inserted by -ftrivial-auto-var-init.\nStore size: "
+; YAML-NEXT:   - StoreSize:       '4'
+; YAML-NEXT:   - String:          ' bytes.'
+; YAML-NEXT:   - String:          ' Volatile: '
+; YAML-NEXT:   - StoreVolatile:       'false'
+; YAML-NEXT:   - String:          .
+; YAML-NEXT:   - String:          ' Atomic: '
+; YAML-NEXT:   - StoreAtomic:       'false'
+; YAML-NEXT:   - String:          .
 ; YAML-NEXT: ...
   %dst = alloca i32
   store i32 0, i32* %dst, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca through a GEP.
 define void @store_alloca_gep() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
   %dst = alloca i32
   %gep = getelementptr i32, i32* %dst, i32 0
   store i32 0, i32* %gep, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca through a GEP in an array.
 define void @store_alloca_gep_array() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
   %dst = alloca [2 x i32]
   %gep = getelementptr [2 x i32], [2 x i32]* %dst, i64 0, i64 0
   store i32 0, i32* %gep, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca through a bitcast.
 define void @store_alloca_bitcast() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
   %dst = alloca [2 x i16]
   %bc = bitcast [2 x i16]* %dst to i32*
   store i32 0, i32* %bc, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca that has a DILocalVariable
 ; attached.
 define void @store_alloca_di() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
   %dst = alloca i32
   store i32 0, i32* %dst, !annotation !0, !dbg !DILocation(scope: !4)
   call void @llvm.dbg.declare(metadata i32* %dst, metadata !6, metadata !DIExpression()), !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to an alloca that has more than one
 ; DILocalVariable attached.
 define void @store_alloca_di_multiple() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
   %dst = alloca i32
   store i32 0, i32* %dst, !annotation !0, !dbg !DILocation(scope: !4)
   call void @llvm.dbg.declare(metadata i32* %dst, metadata !6, metadata !DIExpression()), !dbg !DILocation(scope: !4)
   call void @llvm.dbg.declare(metadata i32* %dst, metadata !7, metadata !DIExpression()), !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to a PHI node that can be two different
 ; allocas.
 define void @store_alloca_phi() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
 entry:
   %dst = alloca i32
   %dst2 = alloca i32
   %cmp = icmp eq i32 undef, undef
   br i1 %cmp, label %l0, label %l1
 l0:
   br label %l2
 l1:
   br label %l2
 l2:
   %phidst = phi i32* [ %dst, %l0 ], [ %dst2, %l1 ]
   store i32 0, i32* %phidst, !annotation !0, !dbg !DILocation(scope: !4)
   ret void
 }
 
 ; Emit a remark that reports a store to a PHI node that can be two different
 ; allocas, where one of it has multiple DILocalVariable.
 define void @store_alloca_phi_di_multiple() {
-; CHECK-NEXT: Initialization inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store inserted by -ftrivial-auto-var-init.
+; CHECK-NEXT: Store size: 4 bytes.
 entry:
   %dst = alloca i32
   %dst2 = alloca i32
   call void @llvm.dbg.declare(metadata i32* %dst, metadata !6, metadata !DIExpression()), !dbg !DILocation(scope: !4)
   call void @llvm.dbg.declare(metadata i32* %dst, metadata !7, metadata !DIExpression()), !dbg !DILocation(scope: !4)
   %cmp = icmp eq i32 undef, undef
   br i1 %cmp, label %l0, label %l1
 l0: