I just want to be clear I was asking a question, I don't really care one way or another, I would just rather not see code churn if it doesn't actually buy us anything.
Dylan On Mon, Feb 09, 2015 at 10:57:31AM -0500, Jan Vesely wrote: > 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 <jan.ves...@rutgers.edu> > > >>>> wrote: > > >>>>> On Fri, 2014-12-12 at 13:37 -0800, Matt Turner wrote: > > >>>>>> On Fri, Dec 12, 2014 at 1:23 PM, Jan Vesely <jan.ves...@rutgers.edu> > > >>>>>> 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 <jan.ves...@rutgers.edu> > > > > > > > > > > > >> _______________________________________________ > > >> Piglit mailing list > > >> Piglit@lists.freedesktop.org > > >> 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 > > >> Piglit@lists.freedesktop.org > > >> 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 <jan.ves...@rutgers.edu> > From 4634b6f72f5b10d66c1a4e2f5e506c1e14143150 Mon Sep 17 00:00:00 2001 > From: Jan Vesely <jan.ves...@rutgers.edu> > Date: Thu, 4 Dec 2014 14:10:50 -0500 > Subject: [Piglit][PATCH 1/1] Error on MSVC compatibility warnings > > Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu> > --- > 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 >
signature.asc
Description: Digital signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit