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

2022-06-27 Thread Vang Thao via Phabricator via cfe-commits 
vangthao updated this revision to Diff 440447.
vangthao added a comment.

Remove "" delimiter. Change ScratchSize [bytes/thread] to ScratchSize 
[bytes/lane]. Use lambda expression to emit remarks. Do not output yaml if 
specific remark is not enabled. Add indentation to make it easier to tell which 
resource usage remark belong to which kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

Files:
  clang/test/Frontend/amdgcn-machine-analysis-remarks.cl
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
  llvm/lib/Target/AMDGPU/SIProgramInfo.h
  llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll

Index: llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
@@ -0,0 +1,158 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -pass-remarks-output=%t -pass-remarks-analysis=kernel-resource-usage -filetype=obj -o /dev/null %s 2>&1 | FileCheck -check-prefix=STDERR %s
+; RUN: FileCheck -check-prefix=REMARK %s < %t
+
+; STDERR: remark: foo.cl:27:0: Kernel Name: test_kernel
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs: 24
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs: 9
+; STDERR-NEXT: remark: foo.cl:27:0: AGPRs: 43
+; STDERR-NEXT: remark: foo.cl:27:0: ScratchSize [bytes/lane]: 0
+; STDERR-NEXT: remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: LDS Size [bytes/block]: 512
+
+; REMARK-LABEL: --- !Analysis
+; REMARK: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:KernelName
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Kernel Name: '
+; REMARK-NEXT:   - KernelName:  test_kernel
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumSGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs: '
+; REMARK-NEXT:   - NumSGPR: '24'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumVGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs: '
+; REMARK-NEXT:   - NumVGPR: '9'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumAGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'AGPRs: '
+; REMARK-NEXT:   - NumAGPR: '43'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:ScratchSize
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'ScratchSize [bytes/lane]: '
+; REMARK-NEXT:   - ScratchSize: '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:Occupancy
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Occupancy [waves/SIMD]: '
+; REMARK-NEXT:   - Occupancy:   '5'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:SGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs Spill: '
+; REMARK-NEXT:   - SGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:VGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs Spill: '
+; REMARK-NEXT:   - VGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:BytesLDS
+; REMARK-NEXT: DebugLoc:{ File: foo.

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

2022-07-06 Thread Vang Thao via Phabricator via cfe-commits 
vangthao marked 5 inline comments as done.
vangthao added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-04-21 Thread Vang Thao via Phabricator via cfe-commits 
vangthao updated this revision to Diff 424344.
vangthao added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Move remarks into its own function. Skip if !ORE. Add clang frontend test. 
Remove LDSSpillSize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

Files:
  clang/test/Frontend/amdgcn-machine-analysis-remarks.cl
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
  llvm/lib/Target/AMDGPU/SIProgramInfo.h
  llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll

Index: llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
@@ -0,0 +1,169 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -pass-remarks-output=%t -pass-remarks-analysis=kernel-resource-usage -filetype=obj -o /dev/null %s 2>&1 | FileCheck -check-prefix=STDERR %s
+; RUN: FileCheck -check-prefix=REMARK %s < %t
+
+; STDERR: remark: foo.cl:27:0: Kernel Name: test_kernel
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs: 24
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs: 9
+; STDERR-NEXT: remark: foo.cl:27:0: AGPRs: 43
+; STDERR-NEXT: remark: foo.cl:27:0: ScratchSize [bytes/thread]: 0
+; STDERR-NEXT: remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: LDS Size [bytes/block]: 512
+; STDERR-NEXT: remark: foo.cl:27:0: --
+
+; REMARK-LABEL: --- !Analysis
+; REMARK: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:KernelName
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Kernel Name: '
+; REMARK-NEXT:   - KernelName:  test_kernel
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumSGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs: '
+; REMARK-NEXT:   - NumSGPR: '24'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumVGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs: '
+; REMARK-NEXT:   - NumVGPR: '9'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumAGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'AGPRs: '
+; REMARK-NEXT:   - NumAGPR: '43'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:ScratchSize
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'ScratchSize [bytes/thread]: '
+; REMARK-NEXT:   - ScratchSize: '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:Occupancy
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Occupancy [waves/SIMD]: '
+; REMARK-NEXT:   - Occupancy:   '5'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:SGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs Spill: '
+; REMARK-NEXT:   - SGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:VGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs Spill: '
+; REMARK-NEXT:   - VGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:BytesLDS
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK

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

2022-04-21 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added inline comments.



Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:596
+   MF.getFunction().getSubprogram(), &MF.front())
+   << "--";
+  });

scott.linder wrote:
> This seems like the wrong place to segment the output; could this be made a 
> part of the emitter itself?
> 
> Or maybe better yet, could all of these values be collected into a single 
> remark? That seems to be how e.g. `llvm/lib/CodeGen/RegAllocGreedy.cpp` does 
> it:
> 
> ```
> Pass:regalloc 
>Name:SpillReloadCopies 
>   Function:f  
>   
>Args:  
> - NumSpills:   '1'
>- String:  
> ' spills '
> - TotalSpillsCost: '1.00e+00' 
>- String:  ' total spills cost '   
>   - NumReloads:   
>'1'
>- String:  ' reloads ' 
>   - TotalReloadsCost: '1.00e+00'  
>  - String:
>   ' total reloads cost '  
>   - String:  generated in function
> ```
We also want to output this in a readable format for the frontend. Collecting 
it all into a single remark seems to break the output format since clang seems 
to ignore all newlines from a diagnostic remark.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-04-29 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added inline comments.



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;
+

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-07-14 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added inline comments.



Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1222
+  // printing multiple diagnostic location and diag opts.
+  EmitResourceUsageRemark("KernelName", "Kernel Name",
+  MF.getFunction().getName());

arsenm wrote:
> Why is this kernel name? Do we not emit these for other functions?
We do emit these for other functions. Would this be better off as "Function 
Name" instead of kernel?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-07-14 Thread Vang Thao via Phabricator via cfe-commits 
vangthao updated this revision to Diff 444759.
vangthao added a comment.

Change "Kernel Name" to "Function Name" and rebased patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

Files:
  clang/test/Frontend/amdgcn-machine-analysis-remarks.cl
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
  llvm/lib/Target/AMDGPU/SIProgramInfo.h
  llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll

Index: llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
@@ -0,0 +1,158 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -pass-remarks-output=%t -pass-remarks-analysis=kernel-resource-usage -filetype=obj -o /dev/null %s 2>&1 | FileCheck -check-prefix=STDERR %s
+; RUN: FileCheck -check-prefix=REMARK %s < %t
+
+; STDERR: remark: foo.cl:27:0: Function Name: test_kernel
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs: 24
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs: 9
+; STDERR-NEXT: remark: foo.cl:27:0: AGPRs: 43
+; STDERR-NEXT: remark: foo.cl:27:0: ScratchSize [bytes/lane]: 0
+; STDERR-NEXT: remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: LDS Size [bytes/block]: 512
+
+; REMARK-LABEL: --- !Analysis
+; REMARK: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:FunctionName
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Function Name: '
+; REMARK-NEXT:   - FunctionName:  test_kernel
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumSGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs: '
+; REMARK-NEXT:   - NumSGPR: '24'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumVGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs: '
+; REMARK-NEXT:   - NumVGPR: '9'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumAGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'AGPRs: '
+; REMARK-NEXT:   - NumAGPR: '43'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:ScratchSize
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'ScratchSize [bytes/lane]: '
+; REMARK-NEXT:   - ScratchSize: '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:Occupancy
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Occupancy [waves/SIMD]: '
+; REMARK-NEXT:   - Occupancy:   '5'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:SGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs Spill: '
+; REMARK-NEXT:   - SGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:VGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs Spill: '
+; REMARK-NEXT:   - VGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:BytesLDS
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'LDS Size [bytes/block]: '
+; REMARK-NEXT:   - BytesLDS:'512'
+; REMARK-N

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

2022-07-14 Thread Vang Thao via Phabricator via cfe-commits 
vangthao updated this revision to Diff 444844.
vangthao added a comment.

Change `auto &&Argument` to `auto Argument`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

Files:
  clang/test/Frontend/amdgcn-machine-analysis-remarks.cl
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
  llvm/lib/Target/AMDGPU/SIProgramInfo.h
  llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll

Index: llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
@@ -0,0 +1,158 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -pass-remarks-output=%t -pass-remarks-analysis=kernel-resource-usage -filetype=obj -o /dev/null %s 2>&1 | FileCheck -check-prefix=STDERR %s
+; RUN: FileCheck -check-prefix=REMARK %s < %t
+
+; STDERR: remark: foo.cl:27:0: Function Name: test_kernel
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs: 24
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs: 9
+; STDERR-NEXT: remark: foo.cl:27:0: AGPRs: 43
+; STDERR-NEXT: remark: foo.cl:27:0: ScratchSize [bytes/lane]: 0
+; STDERR-NEXT: remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: LDS Size [bytes/block]: 512
+
+; REMARK-LABEL: --- !Analysis
+; REMARK: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:FunctionName
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Function Name: '
+; REMARK-NEXT:   - FunctionName:  test_kernel
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumSGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs: '
+; REMARK-NEXT:   - NumSGPR: '24'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumVGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs: '
+; REMARK-NEXT:   - NumVGPR: '9'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumAGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'AGPRs: '
+; REMARK-NEXT:   - NumAGPR: '43'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:ScratchSize
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'ScratchSize [bytes/lane]: '
+; REMARK-NEXT:   - ScratchSize: '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:Occupancy
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Occupancy [waves/SIMD]: '
+; REMARK-NEXT:   - Occupancy:   '5'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:SGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs Spill: '
+; REMARK-NEXT:   - SGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:VGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs Spill: '
+; REMARK-NEXT:   - VGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:BytesLDS
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'LDS Size [bytes/block]: '
+; REMARK-NEXT:   - BytesLDS:'512'
+; REMARK-NEXT: ...
+
+@l

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

2022-07-15 Thread Vang Thao via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67357739c6d3: [AMDGPU] Add remarks to output some resource 
usage (authored by vangthao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

Files:
  clang/test/Frontend/amdgcn-machine-analysis-remarks.cl
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
  llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.h
  llvm/lib/Target/AMDGPU/SIProgramInfo.h
  llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll

Index: llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/resource-optimization-remarks.ll
@@ -0,0 +1,158 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -pass-remarks-output=%t -pass-remarks-analysis=kernel-resource-usage -filetype=obj -o /dev/null %s 2>&1 | FileCheck -check-prefix=STDERR %s
+; RUN: FileCheck -check-prefix=REMARK %s < %t
+
+; STDERR: remark: foo.cl:27:0: Function Name: test_kernel
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs: 24
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs: 9
+; STDERR-NEXT: remark: foo.cl:27:0: AGPRs: 43
+; STDERR-NEXT: remark: foo.cl:27:0: ScratchSize [bytes/lane]: 0
+; STDERR-NEXT: remark: foo.cl:27:0: Occupancy [waves/SIMD]: 5
+; STDERR-NEXT: remark: foo.cl:27:0: SGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: VGPRs Spill: 0
+; STDERR-NEXT: remark: foo.cl:27:0: LDS Size [bytes/block]: 512
+
+; REMARK-LABEL: --- !Analysis
+; REMARK: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:FunctionName
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Function Name: '
+; REMARK-NEXT:   - FunctionName:  test_kernel
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumSGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs: '
+; REMARK-NEXT:   - NumSGPR: '24'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumVGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs: '
+; REMARK-NEXT:   - NumVGPR: '9'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:NumAGPR
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'AGPRs: '
+; REMARK-NEXT:   - NumAGPR: '43'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:ScratchSize
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'ScratchSize [bytes/lane]: '
+; REMARK-NEXT:   - ScratchSize: '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:Occupancy
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'Occupancy [waves/SIMD]: '
+; REMARK-NEXT:   - Occupancy:   '5'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:SGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'SGPRs Spill: '
+; REMARK-NEXT:   - SGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:VGPRSpill
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - String:  'VGPRs Spill: '
+; REMARK-NEXT:   - VGPRSpill:   '0'
+; REMARK-NEXT: ...
+; REMARK-NEXT: --- !Analysis
+; REMARK-NEXT: Pass:kernel-resource-usage
+; REMARK-NEXT: Name:BytesLDS
+; REMARK-NEXT: DebugLoc:{ File: foo.cl, Line: 27, Column: 0 }
+; REMARK-NEXT: Function:test_kernel
+; REMARK-NEXT: Args:
+; REMARK-NEXT:

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

2022-05-11 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added a comment.

I am not sure if allowing clang to accept newlines is a good idea. It seems 
like clang wants to know what type of message is being outputted. For example 
whether this is a remark, warning, etc. but allowing for a diagnostic to output 
their own newline makes it ambiguous where exactly that output is coming from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126908: [VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

2022-10-17 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added inline comments.



Comment at: clang/test/SemaCXX/references.cpp:93
 
-struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due 
to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
+struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due 
to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
 

jkorous wrote:
> Can you please explain in detail what bug are you fixing?
> In my understanding if we stop parsing after the last newline then the 
> existing test would have failed. The difference seems to be only the 
> white-space.
> Am I missing something?
>In my understanding if we stop parsing after the last newline then the 
>existing test would have failed.
The reason it does not fail is because we do not perform an exact match. We 
only do a partial match since `contains()` is used here when matching 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#L102.

We parsed the following from the expected-warning check:
  direct base 'A' is inaccessible due to ambiguity:
  struct C -> B -> A

and the actual output from clang is:
  direct base 'A' is inaccessible due to ambiguity:
  struct C -> B -> A
  struct C -> A

The output from clang does contain the expected warning that we parsed but not 
all of it since we discarded the last line of `\nstruct C -> struct A` from the 
check. You can verify that by adding anything on this line. It will not fail 
the test because we are not checking for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126908

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126908: [VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

2022-06-15 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added a comment.

Ping..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126908

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-15 Thread Vang Thao via Phabricator via cfe-commits 
vangthao created this revision.
Herald added a subscriber: martong.
Herald added a project: All.
vangthao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Accept newlines when the diagnostic string is being given by an outside source
instead of ignoring newlines. I believe the other processing done in
`FormatDiagnostic()` does not ignore newlines so we should also not ignore it
here.

Also format diagnostic options on the first line when there are multiple lines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127923

Files:
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Frontend/TextDiagnosticPrinter.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/test/Analysis/padding_message.cpp

Index: clang/test/Analysis/padding_message.cpp
===
--- clang/test/Analysis/padding_message.cpp
+++ clang/test/Analysis/padding_message.cpp
@@ -1,11 +1,11 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
 
 // expected-warning@+7{{\
-Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct IntSandwich {
   char c1;
@@ -14,11 +14,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct TurDuckHen {
   char c1;
@@ -29,15 +29,15 @@
 #pragma pack(push)
 #pragma pack(2)
 // expected-warning@+11{{\
-Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal). \
-Optimal fields order: \
-i1, \
-i2, \
-i3, \
-c1, \
-c2, \
-c3, \
-c4, \
+Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i1, \n\
+i2, \n\
+i3, \n\
+c1, \n\
+c2, \n\
+c3, \n\
+c4, \n\
 }}
 struct SmallIntSandwich {
   char c1;
@@ -57,11 +57,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-u, \
-c1, \
-c2, \
+Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+u, \n\
+c1, \n\
+c2, \n\
 }}
 struct HoldsAUnion {
   char c1;
@@ -78,11 +78,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-m, \
-s, \
-s2, \
+Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+m, \n\
+s, \n\
+s2, \n\
 }}
 struct StructSandwich {
   struct SmallCharArray s;
@@ -91,11 +91,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 typedef struct {
   char c1;
@@ -104,11 +104,11 @@
 } TypedefSandwich;
 
 // expected-warning@+7{{\
-Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct StructAttrAlign {
   char c1;
@@ -117,12 +117,12 @@
 } __attribute__((aligned(8)));
 
 // expected-warning@+8{{\
-Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal). \
-Optimal fields order: \
-c, \
-c1, \
-c2, \
-x, \
+Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+c, \n\
+c1, \n\
+c2, \n\
+x, \n\
 }}
 struct OverlyAlignedChar {
   char c1;
@@ -132,11 +132,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal). \
-Optimal fields order: \
-o, \
-c1, \
-c2, \
+Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+o, \n\
+c1, \n\
+c2, \n\
 }}
 struct HoldsOverlyAlignedChar {
   char c1;
@@ -146,11 +146,11 @@
 
 void internalStructFunc() {
   // expected-warning@+7{{\
-Excess

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-16 Thread Vang Thao via Phabricator via cfe-commits 
vangthao updated this revision to Diff 437593.
vangthao added a comment.

Update commit message to include the motivation behind the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127923

Files:
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Frontend/TextDiagnosticPrinter.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/test/Analysis/padding_message.cpp

Index: clang/test/Analysis/padding_message.cpp
===
--- clang/test/Analysis/padding_message.cpp
+++ clang/test/Analysis/padding_message.cpp
@@ -1,11 +1,11 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
 
 // expected-warning@+7{{\
-Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct IntSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct IntSandwich {
   char c1;
@@ -14,11 +14,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct TurDuckHen' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct TurDuckHen {
   char c1;
@@ -29,15 +29,15 @@
 #pragma pack(push)
 #pragma pack(2)
 // expected-warning@+11{{\
-Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal). \
-Optimal fields order: \
-i1, \
-i2, \
-i3, \
-c1, \
-c2, \
-c3, \
-c4, \
+Excessive padding in 'struct SmallIntSandwich' (4 padding bytes, where 0 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i1, \n\
+i2, \n\
+i3, \n\
+c1, \n\
+c2, \n\
+c3, \n\
+c4, \n\
 }}
 struct SmallIntSandwich {
   char c1;
@@ -57,11 +57,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-u, \
-c1, \
-c2, \
+Excessive padding in 'struct HoldsAUnion' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+u, \n\
+c1, \n\
+c2, \n\
 }}
 struct HoldsAUnion {
   char c1;
@@ -78,11 +78,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-m, \
-s, \
-s2, \
+Excessive padding in 'struct StructSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+m, \n\
+s, \n\
+s2, \n\
 }}
 struct StructSandwich {
   struct SmallCharArray s;
@@ -91,11 +91,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'TypedefSandwich' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 typedef struct {
   char c1;
@@ -104,11 +104,11 @@
 } TypedefSandwich;
 
 // expected-warning@+7{{\
-Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-i, \
-c1, \
-c2, \
+Excessive padding in 'struct StructAttrAlign' (10 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+i, \n\
+c1, \n\
+c2, \n\
 }}
 struct StructAttrAlign {
   char c1;
@@ -117,12 +117,12 @@
 } __attribute__((aligned(8)));
 
 // expected-warning@+8{{\
-Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal). \
-Optimal fields order: \
-c, \
-c1, \
-c2, \
-x, \
+Excessive padding in 'struct OverlyAlignedChar' (8185 padding bytes, where 4089 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+c, \n\
+c1, \n\
+c2, \n\
+x, \n\
 }}
 struct OverlyAlignedChar {
   char c1;
@@ -132,11 +132,11 @@
 };
 
 // expected-warning@+7{{\
-Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal). \
-Optimal fields order: \
-o, \
-c1, \
-c2, \
+Excessive padding in 'struct HoldsOverlyAlignedChar' (8190 padding bytes, where 4094 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+o, \n\
+c1, \n\
+c2, \n\
 }}
 struct HoldsOverlyAlignedChar {
   char c1;
@@ -146,11 +146,11 @@
 
 void internalStructFunc() {
   // expected-warning@+7{{\
-Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal). \
-Optimal fields order: \
-t, \
-c1, \
-c2, \
+Excessive padding in 'struct X' (6 padding bytes, where 2 is optimal).  [optin.performance.Padding]\n\
+Optimal fields order: \n\
+t, \n\
+c1, \n\
+c2, \n\
 }}
   struct X {
 char c1;
@@ -162,11 +162,11

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-21 Thread Vang Thao via Phabricator via cfe-commits 
vangthao added a comment.

In D127923#3599451 , @aaron.ballman 
wrote:

> Is there a reason the remarks can't use individual diagnostic emissions to 
> simulate newlines? Or is this perhaps a demonstration that the remarks should 
> not be using the diagnostic engine at all and should be emitting their output 
> to a user-controllable stream (or file)?

An issue with using multiple diagnostic emissions to simulate newlines is that 
there are repeated information such as diagnostic location and diagopts 
reprinted for each line. This creates a lot of noise and makes the output 
harder to read. Accepting newlines in diagnostics would allow for a lot more 
flexibility but I do see your point on how it would also encourage longer 
diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127923

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126908: [VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

2022-06-02 Thread Vang Thao via Phabricator via cfe-commits 
vangthao created this revision.
Herald added a project: All.
vangthao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In expected diagnostic checks containing newlines, anything after the last
newline would be discarded since we stop parsing directly after the last 
newline.
Fix this behavior so that any remaining string after the last newline is
appended instead of ignored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126908

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/SemaCXX/references.cpp


Index: clang/test/SemaCXX/references.cpp
===
--- clang/test/SemaCXX/references.cpp
+++ clang/test/SemaCXX/references.cpp
@@ -90,7 +90,7 @@
   int& okay; // expected-note{{reference member 'okay' will never be 
initialized}}
 };
 
-struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due 
to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
+struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due 
to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
 
 void test7(C& c) {
   A& a1 = c; // expected-error {{ambiguous conversion from derived class 'C' 
to base class 'A':}}
Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -631,11 +631,18 @@
 StringRef Content(ContentBegin, ContentEnd-ContentBegin);
 size_t CPos = 0;
 size_t FPos;
+bool Append = false;
 while ((FPos = Content.find(NewlineStr, CPos)) != StringRef::npos) {
   D.Text += Content.substr(CPos, FPos-CPos);
   D.Text += '\n';
   CPos = FPos + NewlineStr.size();
+  Append = true;
 }
+// Previous while loop did not add line after final newline. If there is
+// more text after the newline, append it here.
+if (Append && CPos != Content.size())
+  D.Text += Content.substr(CPos, Content.size());
+
 if (D.Text.empty())
   D.Text.assign(ContentBegin, ContentEnd);
 


Index: clang/test/SemaCXX/references.cpp
===
--- clang/test/SemaCXX/references.cpp
+++ clang/test/SemaCXX/references.cpp
@@ -90,7 +90,7 @@
   int& okay; // expected-note{{reference member 'okay' will never be initialized}}
 };
 
-struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
+struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}}
 
 void test7(C& c) {
   A& a1 = c; // expected-error {{ambiguous conversion from derived class 'C' to base class 'A':}}
Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -631,11 +631,18 @@
 StringRef Content(ContentBegin, ContentEnd-ContentBegin);
 size_t CPos = 0;
 size_t FPos;
+bool Append = false;
 while ((FPos = Content.find(NewlineStr, CPos)) != StringRef::npos) {
   D.Text += Content.substr(CPos, FPos-CPos);
   D.Text += '\n';
   CPos = FPos + NewlineStr.size();
+  Append = true;
 }
+// Previous while loop did not add line after final newline. If there is
+// more text after the newline, append it here.
+if (Append && CPos != Content.size())
+  D.Text += Content.substr(CPos, Content.size());
+
 if (D.Text.empty())
   D.Text.assign(ContentBegin, ContentEnd);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits