On Fri, 2015-02-06 at 19:46 +0000, Jose Fonseca wrote:
> I haven't tried this sort of code with MSVC 2013 U4, but at least in the 
> past, MSVC 2013 was refusing certain kinds of variable length arrays.
> 
> And Jan's patch is a stepping stone to -Wvla option  when compiling 
> (option which apply to the whole tree, including parts that are not 
> built with MSVC), in order to preemptively spot uses of VLA in places 
> where MSVC would break.
> 
> So let me do the following: I'll see if MSVC 2013 accepts or refuses the 
> VLA code that caused problems the first time, and then we'll take it 
> from there.

thank you. getting rid of -Wvla entirely would be preferable.

Dylan, I have attached a follow up patch that I planned to post after
these get in. I probably should have included it in the original series
to avoid the confusion.

jan

> 
> Jose
> 
> 
> On 06/02/15 19:19, Dylan Baker wrote:
> > Is this actually necessary? I though that we required MSVC 2013 u4,
> > because it has c99 support.
> >
> > Also, dma_buf tests require libdrm_intel, which doesn't exist/work on
> > windows, so I don't think anyone will be building them with msvc anytime
> > soon.
> >
> > Dylan
> >
> > On Fri, Feb 06, 2015 at 11:49:08AM -0500, Jan Vesely wrote:
> >> ping
> >>
> >> On Fri, 2014-12-12 at 19:13 -0500, Jan Vesely wrote:
> >>> On Fri, 2014-12-12 at 14:49 -0800, Matt Turner wrote:
> >>>> On Fri, Dec 12, 2014 at 2:13 PM, Jan Vesely <[email protected]> 
> >>>> wrote:
> >>>>> On Fri, 2014-12-12 at 13:37 -0800, Matt Turner wrote:
> >>>>>> On Fri, Dec 12, 2014 at 1:23 PM, Jan Vesely <[email protected]> 
> >>>>>> wrote:
> >>>>>>> On Fri, 2014-12-12 at 12:58 -0800, Matt Turner wrote:
> >>>>>>>> I'm curious what the motivation for removing variably-sized arrays 
> >>>>>>>> is,
> >>>>>>>> but if I accept that that's a good thing to do then the first patch
> >>>>>>>> makes sense, but I don't understand this one.
> >>>>>>>>
> >>>>>>>> How is a variably-size array different from using alloca()?
> >>>>>>>
> >>>>>>> variable size arrays are a c99 feature not supported by msvc (that's 
> >>>>>>> why
> >>>>>>> there is a warning). I don't know which parts actually do need to 
> >>>>>>> build
> >>>>>>> using msvc, but it seemed like a good idea to reduce warning output 
> >>>>>>> (and
> >>>>>>> improve consistency with code that needs to build using msvc).
> >>>>>>>
> >>>>>>> In the first patch I used alloca+free, because it looked nicer than
> >>>>>>> doing size arithmetic. The other cases allocate byte arrays, and the
> >>>>>>> only difference is that alloca (_alloca) is supported by msvc.
> >>>>>>
> >>>>>> Okay, then this patch doesn't do anything useful, since these tests
> >>>>>> shouldn't be built with MSVC. dma_bufs are a Linux thing.
> >>>>>
> >>>>> yes, I understand that, the point was not to build them using msvc.
> >>>>>
> >>>>> the patch usefulness is in enabling switch Wvla to error instead of
> >>>>> warning. other than that, it just reduces warning output.
> >>>>
> >>>> Ah, I see. Okay.
> >>>>
> >>>> For my own curiosity, does this actually change the compiled code?
> >>>
> >>> Looks like the vla version uses fewer instructions but the code size is
> >>> the same (for -O3).
> >>> I'm using gcc 4.9.2 that comes with F21
> >>>
> >>> I have attached release and debug versions of
> >>> ext_image_dma_buf_import-ownership_transfer piglit_display()
> >>> if you're interested
> >>>
> >>> jan
> >>>
> >>>
> >>
> >> --
> >> Jan Vesely <[email protected]>
> >
> >
> >
> >> _______________________________________________
> >> Piglit mailing list
> >> [email protected]
> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_piglit&d=AwIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=axDfDuzBMbe8puZjmv2H6-wYbajqcdPo4tsJ4fdm6is&s=FICxtLdz8KWw9tX2yLTiJHEYEof0TPKcUz25ze35mTc&e=
> >>
> >>
> >> _______________________________________________
> >> Piglit mailing list
> >> [email protected]
> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_piglit&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=axDfDuzBMbe8puZjmv2H6-wYbajqcdPo4tsJ4fdm6is&s=FICxtLdz8KWw9tX2yLTiJHEYEof0TPKcUz25ze35mTc&e=
> 

-- 
Jan Vesely <[email protected]>
From 4634b6f72f5b10d66c1a4e2f5e506c1e14143150 Mon Sep 17 00:00:00 2001
From: Jan Vesely <[email protected]>
Date: Thu, 4 Dec 2014 14:10:50 -0500
Subject: [Piglit][PATCH 1/1] Error on MSVC compatibility warnings

Signed-off-by: Jan Vesely <[email protected]>
---
 CMakeLists.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 12323f0..e218a48 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -212,13 +212,13 @@ if (NOT MSVC)
 	# Unfortunately MSVC does not support C99.  Among all features enabled
 	# by C99, declarations after statements is the most frequently used.
 	# For portability sake, we request gcc to warn when this is used.
-	CHECK_C_COMPILER_FLAG("-Wdeclaration-after-statement" C_COMPILER_FLAG_WDECL_AFTER_STMT)
+	CHECK_C_COMPILER_FLAG("-Werror=declaration-after-statement" C_COMPILER_FLAG_WEDECL_AFTER_STMT)
 	IF (C_COMPILER_FLAG_WDECL_AFTER_STMT)
-		SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wdeclaration-after-statement")
+		SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror=declaration-after-statement")
 	ENDIF ()
-	CHECK_C_COMPILER_FLAG("-Wvla" C_COMPILER_FLAG_WVLA)
-	IF (C_COMPILER_FLAG_WVLA)
-		SET (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wvla")
+	CHECK_C_COMPILER_FLAG("-Werror=vla" C_COMPILER_FLAG_WEVLA)
+	IF (C_COMPILER_FLAG_WEVLA)
+		SET (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror=vla")
 	ENDIF ()
 
 	CHECK_C_COMPILER_FLAG("-Wno-deprecated-declarations" C_COMPILER_FLAG_WNO_DEPRECATED_DECLARATIONS)
-- 
2.1.0

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to