[PATCH] D53408: [PPC64] Long branch thunks.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: ELF/Thunks.cpp:618
+  uint16_t OffHa = (Offset + 0x8000) >> 16;
+  uint16_t OffLo = Offset;
 
----------------
ruiu wrote:
> sfertile wrote:
> > MaskRay wrote:
> > > With `-DLLVM_ENABLE_WARNINGS=on` & MSVC, `/W4` is turned on which warn
> > > `uint16_t OffLo = Offset;`
> > > 
> > > `warning C4244: 'initializing': conversion from 'int64_t' to 'uint16_t', possible loss of data`
> > > 
> > > 
> > > Append `& 0xffff` to suppress it.
> > Thanks, I don't ever compile with MSVC so I didn't realize it would warn on this. I believe I've added similar code in previous patches,  I'll find them and clean those up in a followup NFC patch.
> Hmm, but that narrowing conversion is obvious and we do that everywhere (in particular from int to char because RHS is usually promoted to int due to the usual integer promotion). If MSVC warns on it, we need this, but I don't like that warning in the first place...
I pointed out this because I saw some `& 0xffff` used otherwhere... and not sure if warnings would make some MSVC build bots unhappy..

`/W3` does not warn implicit `int -> char` but `/W4` does. I think it may be acceptable for a warning which is only enabled on `/W4`... Need some Windows expert to confirm this.

`long long -> unsigned` `unsigned long long -> unsigned` is warned in `/W3`.
`http://www.brianlheim.com/2018/04/09/cmake-cheat-sheet.html` says `/W3` is the default 


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53408