[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan abandoned this revision.
yvvan added a comment.

Can't reproduce an error anymore


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

I've tried once again to make a clean build and it worked... :)
And I can't imagine what kind of configuration issue could it be because I 
always use quite the same one :)


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I still don't understand why this is breaking your build.

Assuming this is the line the first error refers to:

  for (unsigned BinOp : {G_ADD, G_SUB, G_MUL, G_AND, G_OR, G_XOR, G_SHL}) {

I don't see how converting G_ADD, even if it is an int, to unsigned would be a 
narrowing conversion.

I did a fresh checkout of LLVM and built with VS 19.00.2415.1 for x64:

  > svn export http://llvm.org/svn/llvm-project/llvm/trunk llvmtest
  > mkdir llvmtest\build
  > cd llvmtest\build
  > cmake -GNinja -DCMAKE_BUILD_TYPE=Release ..
  -- The C compiler identification is MSVC 19.0.24215.1
  -- The CXX compiler identification is MSVC 19.0.24215.1
  -- The ASM compiler identification is MSVC
  ...
  > ninja
  ...

And everything built fine.

There must be something different with your configuration.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In https://reviews.llvm.org/D34279#784089, @hans wrote:

> I have 19.00.24210, so slightly earlier I suppose, but I believe we use 
> Update 3 on our Chromium buildbots, and they seem happy.
>
> Can you paste the full error message? The part I see in your screenshot 
> doesn't really make sense. Why should converting 1 to unsigned int be a 
> narrowing conversion? I'm just trying to understand why we need this.


You can look at the file from screenshot and line numbers inside it - there's 
an implicit conversion from the enumeration TargetOpcode (which is signed int 
for my compiler without my change) to the unsigned int.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I have 19.00.24210, so slightly earlier I suppose, but I believe we use Update 
3 on our Chromium buildbots, and they seem happy.

Can you paste the full error message? The part I see in your screenshot doesn't 
really make sense. Why should converting 1 to unsigned int be a narrowing 
conversion? I'm just trying to understand why we need this.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

btw my cl version is 19.00.24215.1 (MS VS 2015 Update 3). Do you use the same 
one?


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

I've checked once again. Werror is disabled and no /WX flag is used. So this is 
a compiler error in case of msvc even without these flags.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

I also did not have any local patches applied - just checked out release_40 
branch for llvm and clang.
I believe I also don't have warnings as errors enabled. At least I've checked 
for the extra flags and did not find such. I'll search once again if I have /WX 
somewhere in my build config.
But this patch should not break anything in any case.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a subscriber: tstellar.
hans added a comment.

That's strange. I build the llvm 4.0 branch (and trunk too, which has the same 
code) with Visual Studio 2015 without problems. (I don't build with warnings as 
errors though, so if this is a warning maybe I missed it.) Do you have any 
local patches that might cause this problem?

In any case, I don't think Tom is taking more patches for the 4.0.1 release.


https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

error list 



https://reviews.llvm.org/D34279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.

I currently have narrowing conversion error without that fix.
I try building with 64 and 32 bit compilers - both reproduce that error.


https://reviews.llvm.org/D34279

Files:
  include/llvm/Target/TargetOpcodes.h


Index: include/llvm/Target/TargetOpcodes.h
===
--- include/llvm/Target/TargetOpcodes.h
+++ include/llvm/Target/TargetOpcodes.h
@@ -19,7 +19,7 @@
 /// Invariant opcodes: All instruction sets have these as their low opcodes.
 ///
 namespace TargetOpcode {
-enum {
+enum : unsigned {
 #define HANDLE_TARGET_OPCODE(OPC) OPC,
 #define HANDLE_TARGET_OPCODE_MARKER(IDENT, OPC) IDENT = OPC,
 #include "llvm/Target/TargetOpcodes.def"


Index: include/llvm/Target/TargetOpcodes.h
===
--- include/llvm/Target/TargetOpcodes.h
+++ include/llvm/Target/TargetOpcodes.h
@@ -19,7 +19,7 @@
 /// Invariant opcodes: All instruction sets have these as their low opcodes.
 ///
 namespace TargetOpcode {
-enum {
+enum : unsigned {
 #define HANDLE_TARGET_OPCODE(OPC) OPC,
 #define HANDLE_TARGET_OPCODE_MARKER(IDENT, OPC) IDENT = OPC,
 #include "llvm/Target/TargetOpcodes.def"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits