[Mesa-dev] Split version of 07/13 glsl: add double support

Ilia Mirkin imirkin at alum.mit.edu
Thu Feb 5 11:33:39 PST 2015


On Thu, Feb 5, 2015 at 2:26 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Thu, Feb 5, 2015 at 3:05 AM, Topi Pohjolainen
> <topi.pohjolainen at intel.com> wrote:
>> I wanted to try if this could be split into smaller chunks to aid
>> review. Only compile tested (each step compiles).
>
> Thanks a bunch for splitting these. It indeed makes it a ton easier to
> review. I glanced at 07/13 and my eyes glazed over.
>
>> Dave Airlie (17):
>>   glsl: Add double builtin type (was: add double support)
>>   glsl: Add double builtin type generation (was: add double support)
>>   glsl: Uniform linking support for doubles (was: add double support)
>>   glsl/ir: Add builtin function support for doubles (was: add double
>>     support)
>>   glsl/ir: Add printing support for doubles (was: add double support)
>>   glsl/ir: Add cloning support for doubles (was: add double support)
>>   glsl/ir: Add builtin constant function support for doubles
>>   glsl/ir: Add builder support for functions with double floats
>>   glsl: Add support doubles in optimization passes (was: add double
>>     support)
>>   glsl: Add ubo lowering support for doubles (was: add double support)
>>   glsl/ast: Support double floats (was: add double support)
>>   glsl/parser: Support double floats (was: add double support)
>>   glsl/lexer: Support double floats (was: add double support)
>>   glsl: Support double inouts (was: add double support)
>>   glsl: Support double loop control (was: add double support)
>>   glsl: Linking support for doubles (was: add double support)
>
> Please remove the (was: add double support) from the commit summaries
> (some of the patches have it not in the subject but in the message
> body).
>
>>   glsl: add double support
>
> This doesn't seem to have reached my inbox, but I do see it in the archives.
>
> The commit message is wrong. It contains a bunch of stuff about the
> whole 07/13 patch, while the patch itself just adds ir_unop_d2f and
> ir_unop_f2d to a switch statement.
>
> For as much complaining about not getting review on this series as
> I've seen on IRC, I'm kind of disappointed that none of the people who
> have been working on fp64 but aren't authors of these patches put
> their reviews on them. There were lots of things that I pointed out
> that anyone reviewing the patches would have seen.

As the (self-appointed) main complaintant on IRC about this patch
series not having been pushed, the things that you pointed out were
largely _not_ obvious to me (they do mostly make sense once you point
them out though). I'm mostly unfamiliar with the glsl compiler stuff,
and this is a big macro change. A few, of course, were things that I
probably should have caught. But I'm nowhere familiar enough with this
stuff to slap my R-b on it.

I'll integrate Topi's split-up, as well as your feedback, on these patches.

Cheers,

  -ilia


More information about the mesa-dev mailing list