[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Even with newlines forced via extra remarks, I'm not a big fan of the "-----------------------" remark; it doesn't interact well with other random remarks in the output, for example when I enable all remarks using the pattern '.*' I see:

  remark: foo.cl:27:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: test_kernel: MI Instruction count changed from 0 to 6; Delta: 6
  remark: foo.cl:27:0: 0 stack bytes in function
  remark: foo.cl:42:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: test_func: MI Instruction count changed from 0 to 4; Delta: 4
  remark: foo.cl:42:0: 0 stack bytes in function
  remark: foo.cl:42:0: SI insert wait instructions: Function: test_func: MI Instruction count changed from 4 to 5; Delta: 1
  remark: foo.cl:8:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: empty_kernel: MI Instruction count changed from 0 to 1; Delta: 1
  remark: foo.cl:8:0: 0 stack bytes in function
  remark: foo.cl:52:0: AMDGPU DAG->DAG Pattern Instruction Selection: Function: empty_func: MI Instruction count changed from 0 to 1; Delta: 1
  remark: foo.cl:52:0: 0 stack bytes in function
  remark: foo.cl:52:0: SI insert wait instructions: Function: empty_func: MI Instruction count changed from 1 to 2; Delta: 1
  remark: <unknown>:0:0: BasicBlock:
  : 2
  
  remark: foo.cl:27:0: 6 instructions in function
  remark: foo.cl:27:0: Kernel Name: test_kernel
  remark: foo.cl:27:0: SGPRs: 24
  remark: foo.cl:27:0: VGPRs: 9
  remark: foo.cl:27:0: AGPRs: 43
  remark: foo.cl:27:0: ScratchSize [bytes/thread]: 0
  remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
  remark: foo.cl:27:0: SGPRs Spill: 0
  remark: foo.cl:27:0: VGPRs Spill: 0
  remark: foo.cl:27:0: LDS Size [bytes/block]: 512
  remark: foo.cl:27:0: ------------------------------
  remark: <unknown>:0:0: BasicBlock:
  : 2
  
  remark: foo.cl:42:0: 5 instructions in function
  remark: foo.cl:42:0: Kernel Name: test_func
  remark: foo.cl:42:0: SGPRs: 0
  remark: foo.cl:42:0: VGPRs: 0
  remark: foo.cl:42:0: AGPRs: 0
  remark: foo.cl:42:0: ScratchSize [bytes/thread]: 0
  remark: foo.cl:42:0: Occupancy [waves/SIMD]: 0
  remark: foo.cl:42:0: SGPRs Spill: 0
  remark: foo.cl:42:0: VGPRs Spill: 0
  remark: foo.cl:42:0: ------------------------------
  remark: <unknown>:0:0: BasicBlock:
  : 1

If we do keep the delimiter remarks, can we have one at the beginning as well? At least then other remarks don't appear to "bleed" into the new block of remarks.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1178-1266
+
+void AMDGPUAsmPrinter::emitResourceUsageRemarks(
+    const MachineFunction &MF, const SIProgramInfo &CurrentProgramInfo,
+    bool isModuleEntryFunction, bool hasMAIInsts) {
+  if (!ORE)
+    return;
+
----------------
vangthao wrote:
> arsenm wrote:
> > scott.linder wrote:
> > > 
> > I'm sort of surprised the all in one form doesn't come out in something more parsable? It might be worth looking at IR level remarks, since there's probably more usage of them. Are there any existing uses in clang that do something meaningful by parsing out different parts?
> We could check if the specific remark is enabled to avoid cluttering YAML output:
> ```
>   const char *Name = "kernel-resource-usage";
>   LLVMContext &Ctx = MF.getFunction().getContext();
>   if (!Ctx.getDiagHandlerPtr()->isAnalysisRemarkEnabled(Name))
>     return;
> ```
> 
> I do not think using spaces to format the output will work for us. Most of the IR level remarks seems to be related specifically to their pass and/or optimization thus are quite short so readability is not as impacted. We are outputting a decent chunk of information and need some readability for the user.
> 
> Parsing out different parts like this is mostly a workaround for clang ignoring newlines. I am not aware of any other uses doing it like this since many of them are short and uses spaces for formatting.
> Are there any existing uses in clang that do something meaningful by parsing out different parts?

I would expect this sort of thing to happen before it gets serialized to an unstructured string, or for it to happen on a structured output like YAML.

> Parsing out different parts like this is mostly a workaround for clang ignoring newlines.

Can we fix clang to respect newlines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878