On Mon, Feb 8, 2016 at 1:01 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Mon, Feb 8, 2016 at 8:59 AM, Rob Clark <robdcl...@gmail.com> wrote: >> On Mon, Feb 8, 2016 at 6:58 AM, Roland Scheidegger <srol...@vmware.com> >> wrote: >>> Am 06.02.2016 um 22:30 schrieb Marek Olšák: >>>> On Sat, Feb 6, 2016 at 2:45 PM, Rob Clark <robdcl...@gmail.com> wrote: >>>>> On Sun, Jan 31, 2016 at 3:16 PM, Rob Clark <robdcl...@gmail.com> wrote: >>>>>> + // XXX get from pipe_screen? Or just let pipe driver provide? >>>>>> + nir_options.lower_fpow = true; >>>>>> + nir_options.lower_fsat = true; >>>>>> + nir_options.lower_scmp = true; >>>>>> + nir_options.lower_flrp = true; >>>>>> + nir_options.lower_ffract = true; >>>>>> + nir_options.native_integers = true; >>>>>> + >>>>> >>>>> >>>>> btw, one of the few remaining things to tackle is how to handle >>>>> nir_shader_compiler_options struct. To follow the existing approach >>>>> of shader caps, I'd have to add a big pile of caps now, and then keep >>>>> adding them as nir_shader_compiler_options struct grows. Which seems >>>>> sub-optimal. >>>>> >>>>> How do people feel about adding a screen->get_shader_paramp() which, >>>>> along the lines of get_paramf, returns a 'const void *'? Then we >>>>> could add a single cap to return the whole compiler-options struct. >>>>> (And maybe if at some point there was direct support for LLVM as an >>>>> IR, it might need something similar??) >>>>> >>>>> Other possibility is just a pipe->get_nir_compiler_options() type >>>>> hook. A bit more of a point solution, but might make sense if we >>>>> can't think of any other plausible uses for ->get_shader_paramp().. >>>>> and less churn since it would only need to be implemented by drivers >>>>> consuming NIR.. >>>>> >>>>> Thoughts/opinions? >>>> >>>> pipe->get_nir_compiler_options() sounds good. >>>> >>>> Maybe wait for VMWare guys' opinion as well. >>> >>> Looks usable to me, albeit I'm not sure you really need NIR-specific >>> options as such? That is those options above don't really look nir >>> specific - maybe they aren't used with just glsl->tgsi, but it looks to >>> me like they would in theory be applicable to other IR as well. Though I >>> suppose if you just had compiler_otions it would be a bit confusing if >>> you had entries which then may not be used... >> >> Yeah, not really NIR specific (and there are a couple that overlap w/ >> existing caps), other than being used only by NIR.. although it would >> be a lot of churn to keep adding caps when the compiler_options struct >> is extended, and it might be confusing that some of the lowering >> options aren't supported in the TGSI path.. >> >> I guess right now it really only matters for two drivers, and down the >> road I think we won't have more than 3 or 4 drivers using NIR, so I >> suppose it is also an option to start w/ >> screen->get_nir_compiler_options() for now and revisit later. If we >> get to the point where we are always doing glsl->nir and then >> optionally nir->tgsi for drivers that don't consume NIR directly, >> maybe then it would make more sense to expose everything as caps? > > I actually kinda want this for TGSI as well, eventually. Perhaps something > like > > bool get_compiler_options(pipe_shader_ir, void *)
perhaps: const struct pipe_compiler_options * (*get_compiler_options)(struct pipe_screen *, unsigned shader) imo, it should take shader stage as arg so we can have different config per stage, and return a const ptr.. and I think it could directly return options struct (no need for bool return).. I suppose if you plan to add lots of knobs to twiddle for TGSI then shader cap for each would be annoying. Although not super-thrilled about having to translate from generic(ish) pipe struct to nir struct, since the driver will already want a const version of the nir struct for the tgsi_to_nir case. I guess we could do: const void * (*get_compiler_options)(struct pipe_screen *, unsigned shader, enum pipe_shader_ir type) where the return value could be 'struct tgsi_shader_options *' or 'struct nir_shader_compiler_options *', etc.. hmm.. also not sure how to roll that out without a flag day. Perhaps keep the shader params for now (for tgsi) with a helper to populate a tgsi_compiler_options struct for drivers where screen->get_compiler_options() is null.. (and then what about other st's?) BR, -R > or perhaps struct pipe_compiler_options * (which would contain some > common stuff + a union of the per-ir options) instead of the void * > would make more sense? The reason I'm interested is to be able to > indicate that various frontend opts should be disabled. Also it could > be used to get rid of a bunch of PIPE_CAP_TGSI_XYZ's, which are a huge > pain to add all the time. > > -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev