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

Sriraman Tallam via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 26 13:41:29 PDT 2020


tmsriram marked an inline comment as done.
tmsriram added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/basicblock-sections-cfi.ll:4-10
+; From:
+; int foo(int a) {
+;   if (a > 20)
+;     return 2;
+;   else
+;     return 0;
+; }
----------------
dblaikie wrote:
> I /might/ find this simpler:
> 
> ```
> void f1();
> void f2();
> void f3(bool b) {
>   if (b)
>     f1();
>   else
>     f2();
> }
> ```
> 
> But no big deal either way - maybe some more exposition on why 1 CIE and 4FDEs are expected? (I guess 1 CIE shares some common data across the 4 FDEs which are for the 4 basic blocks? Could it be fewer, what about "void f1(); void f2(bool b) { if (b)  f1(); }" is that still adequate to test this functionality?)
@amharc 

Thanks for taking a look, I will change the code.

The FDEs cannot be fewer, one is needed per section.  This is because FDEs do not support ranges like debug info and it is only low-high PC.  In theory, each section could be in arbitrary non-contiguous locations and hence conservatively we need four.

We have a CFI dedup that will reduce the size of each FDE and move the common parts out to the CIE.  I split that out of this patch to keep this simple.


> Could it be fewer, what about "void f1(); void f2(bool b) { if (b) f1(); }" is that still adequate to test this  functionality?)

This will not capture the deduping well because we need atleast 4 basic blocks to have duplicates.  The entry and the exit block have other CFI instructions.




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

https://reviews.llvm.org/D79978





More information about the llvm-commits mailing list