[Mesa-dev] [PATCH] nir/builder: fix C90 build errors
Rob Clark
robdclark at gmail.com
Sat Dec 19 18:52:10 PST 2015
On Sat, Dec 19, 2015 at 9:30 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Sat, Dec 19, 2015 at 9:18 PM, Rob Clark <robdclark at gmail.com> wrote:
>> Note that this is *only* about the header files.. not the src files.
>> I'm not proposing to make NIR support C90.
>
> Why would you need to only make the header filef C90 compliant? If you
> just need to pass around a nir_shader * or something, you can just use
> a forward declaration.
turns out we happen to want structs, fxn prototypes, etc.. ie. what
happens to be in the header ;-)
I guess there is possibly a way to make it so that we build certain
files w/ different warning flags or something (but dropping -Werror
seems like the wrong thing). At any rate, that sounds harder than a
trivial few line patch. And as long as it is trivial to keep the nir
headers C90-clean, that seems like the better thing to do. I mean, we
already take some precautions in the header files to keep them usable
from c++..
>>
>> I will at some point, before it is ready to merge, need to arrange the
>> NIR related bits in mesa st so that we can build without it, for
>> benefit of the MSVC folks.
>>
>> (It might be useful someday to use NIR more extensively in mesa st,
>> and use nir->tgsi pass, so we can do all the opt passes in NIR..
>> although that is *way* more ambitious than what I want to do right
>> now. With any luck, by the time we get to that point, we can rely on
>> a less braindead version of MSVC?)
>
> My understanding is that mesa/st doesn't need to be built with old
> MSVC, just gallium.
Oh, ok, if that is in fact the case, it simplifies some things. Tbh
I'm not 100% clear on what parts are used on windows..
But like I said, if it is just trivial things to keep the nir headers
C90-clean, then why not just do that? I mean, if you have something
planned that would actually make it a burden to keep the headers
C90-clean we can revisit, but I can't really imagine what that would
be. Usually the static-inline stuff tends to be relatively simple
stuff (since you aren't wanting to duplicate a lot of complex stuff in
many object files).
BR,
-R
>>
>> On Sat, Dec 19, 2015 at 8:58 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>> We haven't allowed NIR in core gallium before, since core gallium has
>>> to be built with some old version of MSVC that doesn't support many
>>> C99 features that we really wanted to use. The only reason that
>>> -Werror exists is for compatibility with old MSVC, and if you want to
>>> use NIR with something that needs to build with old MSVC, there are
>>> going to be much bigger changes needed, and we'd rather avoid that. If
>>> you just want to add some NIR-specific stuff that e.g. softpipe
>>> doesn't need to compile against, then you should fix the build system
>>> not to add the warning.
>>>
>>> On Sat, Dec 19, 2015 at 5:39 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>
>>>> We are going to start using nir_builder.h from some gallium code, which
>>>> is currently only C90. Which results in:
>>>>
>>>> In file included from nir/nir_emulate.c:26:0:
>>>> ../../../src/glsl/nir/nir_builder.h: In function ‘nir_build_alu’:
>>>> ../../../src/glsl/nir/nir_builder.h:132:4: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
>>>> unsigned num_components = op_info->output_size;
>>>> ^
>>>> In file included from nir/nir_emulate.c:26:0:
>>>> ../../../src/glsl/nir/nir_builder.h: In function ‘nir_ssa_for_src’:
>>>> ../../../src/glsl/nir/nir_builder.h:271:4: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
>>>> nir_alu_src alu = { NIR_SRC_INIT };
>>>> ^
>>>> cc1: some warnings being treated as errors
>>>>
>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>> ---
>>>> Not sure if I should just go ahead and push this sort of thing. Or
>>>> if we can start requiring C99 for gallium?
>>>>
>>>> src/glsl/nir/nir_builder.h | 7 +++++--
>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/glsl/nir/nir_builder.h b/src/glsl/nir/nir_builder.h
>>>> index 332bb02..6f30306 100644
>>>> --- a/src/glsl/nir/nir_builder.h
>>>> +++ b/src/glsl/nir/nir_builder.h
>>>> @@ -115,6 +115,8 @@ nir_build_alu(nir_builder *build, nir_op op, nir_ssa_def *src0,
>>>> {
>>>> const nir_op_info *op_info = &nir_op_infos[op];
>>>> nir_alu_instr *instr = nir_alu_instr_create(build->shader, op);
>>>> + unsigned num_components;
>>>> +
>>>> if (!instr)
>>>> return NULL;
>>>>
>>>> @@ -129,7 +131,7 @@ nir_build_alu(nir_builder *build, nir_op op, nir_ssa_def *src0,
>>>> /* Guess the number of components the destination temporary should have
>>>> * based on our input sizes, if it's not fixed for the op.
>>>> */
>>>> - unsigned num_components = op_info->output_size;
>>>> + num_components = op_info->output_size;
>>>> if (num_components == 0) {
>>>> for (unsigned i = 0; i < op_info->num_inputs; i++) {
>>>> if (op_info->input_sizes[i] == 0)
>>>> @@ -265,10 +267,11 @@ nir_channel(nir_builder *b, nir_ssa_def *def, unsigned c)
>>>> static inline nir_ssa_def *
>>>> nir_ssa_for_src(nir_builder *build, nir_src src, int num_components)
>>>> {
>>>> + nir_alu_src alu = { NIR_SRC_INIT };
>>>> +
>>>> if (src.is_ssa && src.ssa->num_components == num_components)
>>>> return src.ssa;
>>>>
>>>> - nir_alu_src alu = { NIR_SRC_INIT };
>>>> alu.src = src;
>>>> for (int j = 0; j < 4; j++)
>>>> alu.swizzle[j] = j;
>>>> --
>>>> 2.5.0
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list