[Piglit] [PATCH 2/3] Use alloca instead of variable length arrays

Dylan Baker baker.dylan.c at gmail.com
Tue Feb 10 10:28:27 PST 2015


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.vesely at 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.vesely at 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.vesely at rutgers.edu>
> > >
> > >
> > >
> > >> _______________________________________________
> > >> Piglit mailing list
> > >> Piglit at 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 at 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.vesely at rutgers.edu>

> From 4634b6f72f5b10d66c1a4e2f5e506c1e14143150 Mon Sep 17 00:00:00 2001
> From: Jan Vesely <jan.vesely at 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.vesely at 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
> 



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150210/8253ff91/attachment.sig>


More information about the Piglit mailing list