On Wed, Apr 15, 2015 at 5:13 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Wed, Apr 15, 2015 at 1:31 PM, Matt Turner <matts...@gmail.com> wrote:
>> On Wed, Apr 1, 2015 at 6:19 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
>>> Instead of the complicated and broken-by-design pile of heuristics we had
>>> before, we now have a straightforward lowering:
>>>
>>> 1) All header sources are copied directly using force_writemask_all and,
>>> since they are guaranteed to be a single register, there are no
>>> force_sechalf issues.
>>>
>>> 2) All non-header sources are copied using the exact same force_sechalf
>>> and saturate modifiers as the LOAD_PAYLOAD operation itself.
>>
>> Let's not do this. Nothing puts a saturate modifier on LOAD_PAYLOAD
>> today, and it is kind of confusing about what it means. Can't we have
>> fbwrites that write depth as well. I wouldn't think we wanted to
>> saturate that.
>
> Sure. I can drop saturate and just assert that it's not set. We do
> want to keep force_sechalf and force_writemask_all though.
I didn't think about those before, but I don't know how a load_payload
could have force_writemask_all set. Have I missed something?
I see that setup_color_payload sets force_sechalf for dual-source
fbwrites -- that's the only case we're going to have force_sechalf
set, right?
That is, the Gen < 6 case is going to be handled by passing 16-channel
sources to load_payload and letting it do compr4?
>> I don't think it buys us anything. If we just run copy propagation
>> after lower_load_payload() we'll get the code we want.
[snip]
>>> + /* The COMPR4 code took care of the first 4 sources. We'll let
>>> + * the regular path handle any remaining sources. Yes, we are
>>> + * modifying the instruction but we're about to delete it so
>>> + * this really doesn't hurt anything.
>>> + */
>>> + inst->header_size += 4;
>>
>> I mean, while the comment is a true statement, why is doing this any
>> better than just...
>>
>>> + }
>>> +
>>> + for (uint8_t i = inst->header_size; i < inst->sources; i++) {
>>
>> ... changing this to inst->header_size + 4?
>
> Because the inst->header_size += 4 is predicated on it being a COMPR4
> destination while the code below handles both the remaining sources
> (in the COMPR4 case) and the regular non-COMPR4 case.
Ahh, right. It'd be fewer lines (no commenting necessary) to just have
a 'start' variable that you set to header_size at the top and +=4
here.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
