On Wed, Jan 24, 2018 at 11:20:07AM +0200, Pohjolainen, Topi wrote: > On Fri, Jan 19, 2018 at 11:54:37AM -0800, Rafael Antognolli wrote: > > Some instructions contain fields that are either an address or a value > > of some type based on the content of other fields, such as clear color > > values vs address. That works fine if these fields are in the less > > significant dword, the lower 32 bits of the address, because they get > > OR'ed with the address. But if they are in the higher 32 bits, they get > > discarded. > > > > On Gen10 we have fields that share space with the higher 16 bits of the > > address too. This commit makes sure those fields don't get discarded. > > > > Signed-off-by: Rafael Antognolli <[email protected]> > > --- > > src/intel/genxml/gen_pack_header.py | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/src/intel/genxml/gen_pack_header.py > > b/src/intel/genxml/gen_pack_header.py > > index e6cea8646ff..e81695e2aea 100644 > > --- a/src/intel/genxml/gen_pack_header.py > > +++ b/src/intel/genxml/gen_pack_header.py > > @@ -486,11 +486,16 @@ class Group(object): > > v_address = "v%d_address" % index > > print(" const uint64_t %s =\n > > __gen_combine_address(data, &dw[%d], values->%s, %s);" % > > (v_address, index, dw.address.name + field.dim, v)) > > - v = v_address > > - > > + if len(dw.fields) > address_count: > > + print(" dw[%d] = %s;" % (index, v_address)) > > + print(" dw[%d] = (%s >> 32) | (%s >> 32);" % (index > > + 1, v_address, v)) > > + continue > > + else: > > + v = v_address > > print(" dw[%d] = %s;" % (index, v)) > > print(" dw[%d] = %s >> 32;" % (index + 1, v)) > > I'm wondering if we could have left the "continue" out and write the > else-branch directly just like we did if: > > print(" dw[%d] = %s;" % (index, v_address)) > print(" dw[%d] = %s >> 32;" % (index + 1, v_address))
Hi Topi, I was rebasing the series on top of master and while applying your suggestion, I just noticed it's not gonna work. Notice that the last 2 lines are executed both when the else branch is taken, or when the outer "if dw.address:" is not taken. If I do as you suggest, that last case won't be covered. I know it looks really ugly, I'll try to think of a better way to do this, but for now I'll just submit the updated series with this version again to get reviews on other stuff. Thanks for the review anyway. > > > > + > > class Value(object): > > def __init__(self, attrs): > > self.name = safe_name(attrs["name"]) > > -- > > 2.14.3 > > > > _______________________________________________ > > mesa-dev mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
