[PATCH] R600/SI: Add comments for number of used registers.

[Tom Stellard]

On Tue, Nov 19, 2013 at 12:14:21PM -0800, Matt Arsenault wrote:
> On 11/18/2013 10:36 AM, Tom Stellard wrote:
> > On Sun, Nov 17, 2013 at 07:35:33PM -0800, Matt Arsenault wrote:
> >>    Use EmitRawText to avoid indentation
> >>
> >> http://llvm-reviews.chandlerc.com/D2206
> >>
> >> CHANGE SINCE LAST DIFF
> >>    http://llvm-reviews.chandlerc.com/D2206?vs=5616&id=5617#toc
> >>
> >> Files:
> >>    lib/Target/R600/AMDGPUAsmPrinter.cpp
> >>    lib/Target/R600/AMDGPUAsmPrinter.h
> >>
> >> Index: lib/Target/R600/AMDGPUAsmPrinter.cpp
> >> ===================================================================
> >> --- lib/Target/R600/AMDGPUAsmPrinter.cpp
> >> +++ lib/Target/R600/AMDGPUAsmPrinter.cpp
> >> @@ -46,8 +46,7 @@
> >>   }
> >>   
> >>   AMDGPUAsmPrinter::AMDGPUAsmPrinter(TargetMachine &TM, MCStreamer &Streamer)
> >> -    : AsmPrinter(TM, Streamer)
> >> -{
> >> +    : AsmPrinter(TM, Streamer) {
> >>     DisasmEnabled = TM.getSubtarget<AMDGPUSubtarget>().dumpCode() &&
> >>                     ! Streamer.hasRawTextSupport();
> >>   }
> >> @@ -56,6 +55,7 @@
> >>   /// the call to EmitFunctionHeader(), which the MCPureStreamer can't handle.
> >>   bool AMDGPUAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
> >>     SetupMachineFunction(MF);
> >> +
> >>     if (OutStreamer.hasRawTextSupport()) {
> >>       OutStreamer.EmitRawText("@" + MF.getName() + ":");
> >>     }
> >> @@ -65,9 +65,12 @@
> >>                                                 ELF::SHT_PROGBITS, 0,
> >>                                                 SectionKind::getReadOnly());
> >>     OutStreamer.SwitchSection(ConfigSection);
> >> +
> >>     const AMDGPUSubtarget &STM = TM.getSubtarget<AMDGPUSubtarget>();
> >> +  SIProgramInfo KernelInfo;
> >>     if (STM.getGeneration() > AMDGPUSubtarget::NORTHERN_ISLANDS) {
> >> -    EmitProgramInfoSI(MF);
> >> +    findNumUsedRegistersSI(MF, KernelInfo.NumSGPR, KernelInfo.NumVGPR);
> >> +    EmitProgramInfoSI(MF, KernelInfo);
> >>     } else {
> >>       EmitProgramInfoR600(MF);
> >>     }
> >> @@ -79,6 +82,14 @@
> >>     OutStreamer.SwitchSection(getObjFileLowering().getTextSection());
> >>     EmitFunctionBody();
> >>   
> >> +  if (isVerbose() && OutStreamer.hasRawTextSupport()) {
> >> +    // TODO: How can we prevent these from being indented?
> >> +    OutStreamer.EmitRawText(
> >> +      Twine("; Kernel info:\n") +
> >> +      "; Max SGPR: " + Twine(KernelInfo.NumSGPR) + "\n" +
> >> +      "; Max VGPR: " + Twine(KernelInfo.NumVGPR) + "\n\n\n");
> >> +  }
> >> +
> > Can you stick this in an ELF section so it can be accessed from Mesa?
> > The section name should be .AMDGPU.csdata and we should try to use the
> > same formatting as the proprietary driver:
> >
> > NumVgprs             = 3;
> > NumSgprs             = 13;
> >
> > Getting the whitespace correct is not important.
> >
> > -Tom
> >
> >
> Why does Mesa need this? I just added this for convenience when reading 
> the ISA. Doesn't mesa use the binary form of these which is already emitted?
>

There is information loss in the binary format.  LDS size, for example is
rounded up to 64 dwords.  The driver needs to know the exact LDS size in
case some of the local memory objects have their size declared outside
of the shader.

GPR usage is also reported in multiples of 4 and 8. I'm not sure if the
driver really needs to know the exact values for GPRs, but if we are
going to start emitting this information, I figure it is better to do it
in a way that Mesa can use if it needs to.  Does putting this
information in an ELF section complicate the patch too much?

-Tom