[Mesa-dev] RFC: help on llvmpipe

[Adhemerval Zanella]

On 01/11/2013 01:05 PM, Michel Dänzer wrote:
> On Fre, 2013-01-11 at 11:53 -0200, Adhemerval Zanella wrote: 
>> I'm trying to make llvmpipe work correctly on PowerPC64. The llvm MCJIT backend
>> works pretty well now, so now I'm focused on the llvmpipe code. I already sent a few
>> patches to address the existing testcases (lp_test_arit, lp_test_blend, lp_test_conv,
>> lp_test_format, lp_test_printf). However I found out that the lp_test_format 
>> is not coded right to take care of the endianess of the architecture, so I had to
>> revert some changes of my two of my patches that do byte-swap in some format handling
>> (the byte-swap in lp_bld_format_aos_array.c and lp_bld_format_aos.c). I'll work on
>> patch to revert the patch and fix the testcase.
> Fix according to what? :) There is no explicit definition of the byte
> order of PIPE_FORMAT_*, and the current code is inconsistent about it I
> think. I'm afraid we need to start at basics like that and work our way
> up from there.

My patches were to correct the lp_test_xxx from llvmpipe source tree, but I noticed later
there is no need to byte swap the loaded data. In fact, by reverting I could actually
see some color (although inverted) on some piglit tests I'm focusing.


>
> https://bugs.freedesktop.org/show_bug.cgi?id=43698 has a possible start
> for this for r300g assuming PIPE_FORMAT_* are always little endian. But
> I suspect it can't be that simple either, e.g. vertex elements probably
> need to use the CPU native byte order, as that's what e.g. GL apps write
> to VBOs. Maybe we'll need variants of at least some formats for both
> byte orders.
>
> I haven't looked into this for llvmpipe in any detail.
>
>
The patch inside the bugzilla gave an idea on try to fix the stores instead of loads,
My first approach was to byte swap the data store at src/gallium/drivers/llvmpipe/lp_state_fs.c,
with the follow patch:

--- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
@@ -86,6 +86,7 @@
 #include "gallivm/lp_bld_pack.h"
 #include "gallivm/lp_bld_format.h"
 #include "gallivm/lp_bld_quad.h"
+#include "gallivm/lp_bld_printf.h"
 
 #include "lp_bld_alpha.h"
 #include "lp_bld_blend.h"
@@ -888,7 +889,7 @@ store_unswizzled_block(struct gallivm_state *gallivm,
 {
    LLVMBuilderRef builder = gallivm->builder;
    unsigned row_size = src_count / block_height;
-   unsigned i;
+   unsigned i, j;
 
    /* Ensure src exactly fits into block */
    assert((block_width * block_height) % src_count == 0);
@@ -909,6 +910,17 @@ store_unswizzled_block(struct gallivm_state *gallivm,
       src_ptr = LLVMBuildGEP(builder, base_ptr, gep, 2, "");
       src_ptr = LLVMBuildBitCast(builder, src_ptr, LLVMPointerType(lp_build_vec_type(gallivm, src_type), 0), "");
 
+      LLVMValueRef undef = LLVMGetUndef(lp_build_vec_type(gallivm, src_type)); 
+      LLVMValueRef shuffles[LP_MAX_VECTOR_WIDTH];
+      for (j = 0; j < src_type.length; j+=4) {
+         shuffles[j+0] = lp_build_const_int32(gallivm, j+3);
+         shuffles[j+1] = lp_build_const_int32(gallivm, j+2);
+         shuffles[j+2] = lp_build_const_int32(gallivm, j+1);
+         shuffles[j+3] = lp_build_const_int32(gallivm, j+0);
+      }
+      src[i] = LLVMBuildShuffleVector(builder, src[i], undef, 
+                                      LLVMConstVector(shuffles, src_type.length), "");
+
       src_ptr = LLVMBuildStore(builder, src[i], src_ptr);
 
       lp_set_store_alignment(src_ptr, src_alignment);


Now, I actually can see the right color on the 'drawpix-z' piglit testcase, however I'm still seeing
some artefact's (purple triangles crossing from upper right bound to lower left bound). I'm not sure
if it the right place to try to adjust the llvmpipe output to architecture memory format, so I'm 
accepting suggestions.