[PATCH] D79978: Call Frame Information (CFI) Handling for Basic Block Sections

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/basicblock-sections-cfiinstr.ll:23-30
+; int compute(bool k, int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8, int p9, int pa, int pb, int pc) {
+;  int result;
+;  if (k)
+;    result = p1 * p2 + p3 / p4 - p5 * p6 + p7 / p8 - p9 * pa + pb / pc;
+;  else
+;    result = p1 / p2 - p3 * p4 + p5 / p6 - p7 * p8 + p9 / pa - pb * pc;
+;  return result;
----------------
tmsriram wrote:
> dblaikie wrote:
> > Seems like a surprisingly large amount of computation - is it there for a reason? needed to push some optimization or layout decisions? Could it all use the same operation (just all multiplication, for instance) or is the different operations significant? (Well, I guess they have to differ between the two branches - but could all be the same within each one?) does it need 12 parameters? Could it be fewer & use a function call?
> > 
> > (etc, etc - simple test case, maybe some comments describing what's significant about the features of it that are needed to demonstrate the desired behavior, etc)
> 
> 
> 
> 
> It was done so that more callee-saved registers are used and when more callee saved registers are used cfi_offset directives are needed for it.  The .s looks like this for a basic block that does the computation:
> 
> _Z7computebiiiiiiiiiiii.1:              # %if.then
> 	.cfi_startproc
> 	.cfi_def_cfa %rbp, 16
> 	.cfi_offset %rbx, -48
> 	.cfi_offset %r12, -40
> 	.cfi_offset %r14, -32
> 	.cfi_offset %r15, -24
> 	.cfi_offset %rbp, -16
> 
> Each basic block that goes in a different section must emit cfi directives for callee-saved registers.  The parameters is to make sure the caller saved registers are taken and the callee saved registers are forced so that we can check that the cfi emission indeed works for callee saved registers. 
> 
Ah, OK - a comment might be handy to describe that?

And rather than the somewhat arbitrary computation, perhaps an opaque function call would suffice? Or would that introduce other complications for spills/saves/etc?

Maybe using a pass by value struct as the parameter type so the long parameter list doesn't have to be repeated?


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

https://reviews.llvm.org/D79978