[PATCH] D100375: [yaml2obj] Enable support for parsing 64-bit XCOFF.

ChenZheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 14 21:44:00 PDT 2021


shchenz added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:287
       }
       if (PaddingSize > 0)
         W.OS.write_zeros(PaddingSize);
----------------
jhenderson wrote:
> Esme wrote:
> > jhenderson wrote:
> > > Didn't notice this before when reviewing the 32-bit implementation: why is this check here? Same comment for other such checks.
> > As Zheng's comment in D95505
> > 
> > 
> > > We don't have to call write_zeros if PaddingSize is 0
> > 
> > 
> You're right that we don't have to, but by adding the guard, you complicate the logic. Calling it is harmless.
> 
> For a similar case, you don't put an `if (!x.empty())` around a loop which iterates over the elements of `x`, for example.
Either way is ok to me. Both ways have their pros and cons.
We used to add a guard for the same cases(`write_zeros()` for padding) in XCOFFObjectWriter.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100375



More information about the llvm-commits mailing list