[llvm-commits] Windows compilation warnings

[Villmow, Micah]

> -----Original Message-----
> From: Dimitry Andric [mailto:dimitry at andric.com]
> Sent: Friday, July 16, 2010 4:26 AM
> To: Villmow, Micah
> Cc: llvm-commits
> Subject: Re: [llvm-commits] Windows compilation warnings
> 
> On 2010-07-16 04:00, Villmow, Micah wrote:
> > This patch set makes Windows build cleanly for 32bit binaries from a
> > 64bit machine.
> 
> Hmm, I regularly build 32-bit binaries on my 64-bit Windows machine,
> but I never get many warnings about truncations, except the recently
> introduced ones about enums being truncated.  It is only when I build
> the x64 project that I get a lot of those warnings.
> 
> Are you compiling with non-standard settings?  E.g. -Wp64 (which I
> remember is deprecated) or -W4, the maximum warning setting?
> 
[Villmow, Micah] 
Here are the options we are compiling with.
  -TC /Z7 /Od /Ob1 /W0 /GR /MTd /Zc:forScope /Zc:wchar_t /wd4996 /favor:blend /TP /arch:SSE2 /D__SSE__ /D__SSE2__ /GS- /fp:except- -c -DWIN32 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_LIB -D_SCL_SECURE_NO_WARNINGS -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS

Not sure if there are anything outrageous here.

> 
> > Some thoughts about what I saw while doing this.
> >
> > size_t is not used in places where it should be, return value of
> > size(), input value of resize().
> 
> Actually, in most cases, the class's "size_type", "difference_type" and
> so on should be used.  AFAIK there is no guarantee these are always
> equivalent to size_t (although they usually are in practice).
> 
> But indeed, the cases where size_types are stuffed into plain ints or
> unsigneds are many, although it may not be that easy to just change
> whatever variable holds them to size_type.  For example, you may need
> to
> change function declarations to effectively do that, and such changes
> tend to "cascade" through a lot of the source.
> 
> 
> > getZExtValue()/getSExtValue() need 32bit explicit versions, this
> > would remove probably 60% of truncation casts.
> 
> But you would still need to go over *all* the invocations, to replace
> them... and be extremely careful not to break anything by those
> replacements.
> 
[Villmow, Micah] 
Yeah, this patch isn't completely valid, some overnight internal testing we had broke, so I'm going to break it out into a sequence of smaller patches for each component. To very each one.

> 
> > uint64_t is overused in places where uint32_t is sufficient.
> 
> There may be reasons for those uses, but the only ones that can explain
> are the authors of the code in question. :)
> 
> 
> > unsigned/signed is used where uint32_t/int32_t should be used.
> >
> > Should there be something the developers notes about these? I think
> > it is better to explicitly specify the integer size instead on
> > relying on the compiler to pick the size.
> 
> I am not sure if hardcoding the number of bits in the type is always
> the
> best solution in all cases.  This really depends on the specific piece
> of code.
> 
> Most of the times you just want to use plain int or unsigned, because
> that will always be the machine's default word.  You only have to pay
> attention when mixing those with "size" types, such as object lengths
> and pointer differences.

[Villmow, Micah] 
Any idea on how I should approach this? I do want to make llvm clean so we can turn on fail on warning internally for our whole build system on windows. Now that I've gone over all of them, it does give me an idea on how to fix them, but outside feedback would be appreciated.

I think I'm going to focus on Tablegen for now, and worry about llvm proper later.

Micah