Keith Whitwell pisze:
On Wed, 2009-11-25 at 08:51 -0800, michal wrote:
michal pisze:
Keith Whitwell pisze:
On Wed, 2009-11-25 at 06:28 -0800, michal wrote:
Keith Whitwell pisze:
I've pushed a feature branch with some tgsi simplifications in it.  With
these I've removed the biggest remaining oddities of that language, and
it's getting to a place where I'm starting to be happy with it as a
foundation for future work.

Most of the surprising stuff like multiple negate flags, etc, is gone
now, and the core tokens are quite a bit easier to understand than in
previous iterations.

I've still got my eye on reducing the verbosity of the names in the
tgsi_parse.h "FullToken" world, and promoting the tgsi_any_token union
into p_shader_tokens.h.

It would be good if people can review the interface changes and provide
feedback, and also test out their drivers on this branch.  I've done
minimal softpipe testing so far but will do more over the next few days.

All looks good to me, I'm happy somebody had the guts to cut off all the cruft in one shot.

I see some compile errors on windows build -- I will fix those along with other minor bugs I have spotted.

Now, looking at the interface, I'm thinking about removing some more tokens.

1) Remove tgsi_dimension and use tgsi_src_register directly with some well-defined constraints.

2) Do the same to tgsi_instruction_predicate. Really, it's just an optional src operand with some restrictions.
Interesting.  I'd be keen to see a patch.


Attached. But the more I look at it the more lame it gets.

Another option would be to define tgsi_any_register that would have File, Index, Indirect and Dimension fields. Then there would be more specialised tgsi_*_register tokens, that would be binary compatible with the first one. One could cast them using a union and avoid more mistakes at compile time. That way we don't have to put the constraints in comments, but be more strict and use the compiler to enforce them. I will follow up with a patch.
Attached.

This makes me wonder about a couple of other things, like whether 16
bits is sufficient for the index value.  Probably its fine, but it's not
beyond belief to consider a constant buffer of 256k or larger.

I'd consider dropping the "generic_register" struct and any idea of a
union of these registers.  I'm not really sure we want to encourage the
idea of people casting between these registers -- for the most part they
should be building these things with ureg-style functions rather than
messing around with the tokens directly.
If you can easily cast between registers, that defeats any static
constraints you attempt to impose via the type system, and you may as
well just use "src_register" for predicates and dimensions.  An
interpreter which might benefit from being able to share some code paths
for the different registers doesn't need the union to be public.

Basically, this looks like a good regularization/cleanup, but let's drop
generic_register and not create any public union of these register
structs.

Attached an updated patch.

One thing to note in general is that by removing the Extended flags and the fact that some of the tokens already use up all the available 32 bits, the only way to extend the language may be by incrementing the version number in shader's header. This can be a good or a bad thing, depending on the direction Gallium is heading, but with a bit of discipline that should be a good thing.
diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
b/src/gallium/include/pipe/p_shader_tokens.h
index 7d73d7d..18eed97 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -290,7 +290,7 @@ union tgsi_immediate_data
  * respectively. For a given operation code, those numbers are fixed and are
  * present here only for convenience.
  *
- * If Predicate is TRUE, tgsi_instruction_predicate token immediately follows.
+ * If Predicate is TRUE, tgsi_predicate_register token immediately follows.
  *
  * Saturate controls how are final results in destination registers modified.
  */
@@ -350,77 +350,88 @@ struct tgsi_instruction_texture
    unsigned Padding  : 24;
 };
 
-/*
- * For SM3, the following constraint applies.
- *   - Swizzle is either set to identity or replicate.
- */
-struct tgsi_instruction_predicate
-{
-   int      Index    : 16; /* SINT */
-   unsigned SwizzleX : 2;  /* TGSI_SWIZZLE_x */
-   unsigned SwizzleY : 2;  /* TGSI_SWIZZLE_x */
-   unsigned SwizzleZ : 2;  /* TGSI_SWIZZLE_x */
-   unsigned SwizzleW : 2;  /* TGSI_SWIZZLE_x */
-   unsigned Negate   : 1;  /* BOOL */
-   unsigned Padding  : 7;
-};
-
 /**
  * File specifies the register array to access.
  *
- * Index specifies the element number of a register in the register file.
+ * Index specifies the register number in the specified register file.
  *
- * If Indirect is TRUE, Index should be offset by the X component of a source
- * register that follows. The register can be now fetched into local storage
- * for further processing.
+ * If Indirect is TRUE, Index should be offset by the tgsi_indirect_register
+ * that follows.
  *
- * If Negate is TRUE, all components of the fetched register are negated.
+ * If Dimension is TRUE, tgsi_dimension_register follows.
  *
- * The fetched register components are swizzled according to SwizzleX, 
SwizzleY,
- * SwizzleZ and SwizzleW.
+ * If Absolute is TRUE, all components of the register get their signs
+ * cleared.
  *
+ * If Negate is TRUE, all components of the register are negated.
+ *
+ * The register components are swizzled according to SwizzleX, SwizzleY,
+ * SwizzleZ and SwizzleW.
  */
 
 struct tgsi_src_register
 {
-   unsigned File        : 4;  /* TGSI_FILE_ */
-   unsigned Indirect    : 1;  /* BOOL */
-   unsigned Dimension   : 1;  /* BOOL */
-   int      Index       : 16; /* SINT */
-   unsigned SwizzleX    : 2;  /* TGSI_SWIZZLE_ */
-   unsigned SwizzleY    : 2;  /* TGSI_SWIZZLE_ */
-   unsigned SwizzleZ    : 2;  /* TGSI_SWIZZLE_ */
-   unsigned SwizzleW    : 2;  /* TGSI_SWIZZLE_ */
-   unsigned Absolute    : 1;    /* BOOL */
-   unsigned Negate      : 1;    /* BOOL */
+   int      Index    : 16; /* SINT */
+   unsigned File     : 4;  /* TGSI_FILE_ */
+   unsigned Indirect : 1;  /* BOOL */
+   unsigned Dimension: 1;  /* BOOL */
+   unsigned Absolute : 1;  /* BOOL */
+   unsigned Negate   : 1;  /* BOOL */
+   unsigned SwizzleX : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleY : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleZ : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleW : 2;  /* TGSI_SWIZZLE_ */
 };
 
 /**
- * If tgsi_src_register::Modifier is TRUE, tgsi_src_register_modifier follows.
- * 
- * Then, if tgsi_src_register::Indirect is TRUE, another tgsi_src_register
- * follows.
- *
- * Then, if tgsi_src_register::Dimension is TRUE, tgsi_dimension follows.
+ * The individual register component updates are controlled by WriteMask.
  */
 
+struct tgsi_dst_register
+{
+   int      Index    : 16; /* SINT */
+   unsigned File     : 4;  /* TGSI_FILE_ */
+   unsigned Indirect : 1;  /* BOOL */
+   unsigned Dimension: 1;  /* BOOL */
+   unsigned WriteMask: 4;  /* TGSI_WRITEMASK_ */
+   unsigned Reserved : 6;
+};
+
+struct tgsi_indirect_register
+{
+   int      Index    : 16; /* SINT */
+   unsigned File     : 4;  /* TGSI_FILE_ */
+   unsigned Indirect : 1;  /* BOOL */
+   unsigned Dimension: 1;  /* BOOL */
+   unsigned SwizzleX : 2;  /* TGSI_SWIZZLE_ */
+   unsigned Reserved : 8;
+};
 
-struct tgsi_dimension
+struct tgsi_dimension_register
 {
-   unsigned Indirect    : 1;  /* BOOL */
-   unsigned Dimension   : 1;  /* BOOL */
-   unsigned Padding     : 14;
-   int      Index       : 16; /* SINT */
+   int      Index    : 16; /* SINT */
+   unsigned Reserved1: 4;
+   unsigned Indirect : 1;  /* BOOL */
+   unsigned Dimension: 1;  /* BOOL */
+   unsigned Reserved2: 10;
 };
 
-struct tgsi_dst_register
+/**
+ * File must be TGSI_FILE_PREDICATE.
+ *
+ * Swizzle* must be either set to identity or replicate.
+ */
+
+struct tgsi_predicate_register
 {
-   unsigned File        : 4;  /* TGSI_FILE_ */
-   unsigned WriteMask   : 4;  /* TGSI_WRITEMASK_ */
-   unsigned Indirect    : 1;  /* BOOL */
-   unsigned Dimension   : 1;  /* BOOL */
-   int      Index       : 16; /* SINT */
-   unsigned Padding     : 6;
+   int      Index    : 16; /* SINT */
+   unsigned File     : 4;  /* TGSI_FILE_PREDICATE */
+   unsigned Reserved : 3;
+   unsigned Negate   : 1;  /* BOOL */
+   unsigned SwizzleX : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleY : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleZ : 2;  /* TGSI_SWIZZLE_ */
+   unsigned SwizzleW : 2;  /* TGSI_SWIZZLE_ */
 };
 
 
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to