[PATCH] D14948: [X86] Use precise CFI for pushes by default

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 10:28:42 PST 2015


On 2 December 2015 at 09:21, Michael Kuperstein
<michael.m.kuperstein at intel.com> wrote:
> mkuper added a comment.
>
> Thanks, Rafael.
>
>
> ================
> Comment at: include/llvm/CodeGen/MachineModuleInfo.h:168
> @@ -167,1 +167,3 @@
>
> +  /// UsePreciseUnwindInfo - True if we should emit precise CFA adjustment
> +  /// information. False if we only need CFA to be precise at call sites.
> ----------------
> rafael wrote:
>> Don't repeat name in comment.
> Following style of surrounding code...

Please use the new style. Or, even better, change the existing code first.

> ================
> Comment at: include/llvm/CodeGen/MachineModuleInfo.h:249
> @@ -242,1 +248,3 @@
> +  bool usePreciseUnwindInfo() const { return UsePreciseUnwindInfo; }
> +  void setUsePreciseUnwindInfo(bool b) { UsePreciseUnwindInfo = b; }
>
> ----------------
> rafael wrote:
>> Nothing calls this, so it is dead code.
>>
>> Do you have a followup patch that uses it?
> Not yet, although one is eventually planned, per the comment in line 176.
> As I wrote in the summary, I can pull out the dead code (and replace it with TODOs), if there's an objection to keeping it in the meanwhile.

Yes, please leave it as TODOs for now.

Cheers,
Rafael


More information about the llvm-commits mailing list