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

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram marked 4 inline comments as done.
tmsriram added a comment.

In D79978#2138208 <https://reviews.llvm.org/D79978#2138208>, @MaskRay wrote:

> I have studied CFIInstrInserter in May. If you don't mind, please give me some time to review as well.
>
> For `basic-block-sections-cfiinstr_1.ll`, have you considered places like `CodeGen/X86/cfi-inserter-*`? You may even create a subdirectory there.
>  `_1` is not very common. `-1` is more common.


Done.

> `curl -L 'https://reviews.llvm.org/D79978?download=1'` does not have a/ or b/ prefix. I think that may be why `arc patch D79978` cannot apply the patch.
>  Can you upload a diff with either `arc diff`, git format-patch -1 or `git diff 'HEAD^'`? Thanks.

Uploaded after git diff HEAD^.  Please see if this works.



================
Comment at: llvm/test/DebugInfo/X86/basic-block-sections-cfiinstr_1.ll:6
+; CFI_INSTR: _Z7computebiiiiii
+; CFI_INSTR: bb.0
+; CFI_INSTR: bb.1
----------------
MaskRay wrote:
> I think these labels may need `:` suffix and a `# `prefix to make them unique.
I added the full name now, not sure what you mean by '#' prefix. 


================
Comment at: llvm/test/DebugInfo/X86/basic-block-sections-cfiinstr_1.ll:20-30
+; Exhaust caller-saved parameter registers and  force callee saved registers to
+; be used in the computation.  This tests that CFI directives for callee saved
+; registers are generated with basic block sections.
+; extern int f1(int, int, int);
+;
+; int compute(bool k, int p1, int p2, int p3, int p4, int p5, int p6) {
+;   int result = p1;
----------------
dblaikie wrote:
> this looks nicer - though I'd still like a bit more commentary on exactly how/why these constructs are here? Why two function calls with interleaved parameters rather than one, etc?
> 
> Mostly I'm hoping the test would explain why these constructs are used and which parts are relevant. (does the function need a non-void return type? or could the function calls be void-returning but conditional? (it's not like they can be optimized away, since they might have side effects anyway))
I cleaned this up a bit more adding more comments and changing it to a void func.  A single func call is not utilizing callee saved registers but using two calls like this is forcing it.  PTAL.


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

https://reviews.llvm.org/D79978