Re: [Mesa-dev] [PATCH] swr: [rasterizer core] Remove dead code Clipper::ClipScalar()

[Vinson Lee]

Tested-by: Vinson Lee 

On Thu, Feb 2, 2017 at 12:42 PM, Cherniak, Bruce
 wrote:
> I followed up with a v2 that includes the bugzilla reference.
>
> Good point, I’ll look into following up with a patch to remove Clip().
>
> Thanks for the quick review.
>
>> On Feb 2, 2017, at 2:26 PM, Ilia Mirkin  wrote:
>>
>> Reviewed-by: Ilia Mirkin 
>>
>> I got confused by this code as well when I was trying to understand
>> the clipper. I think the Clip() function can go too now in the .cpp
>> file (as well as the fwd decl in the header)?
>>
>> On Thu, Feb 2, 2017 at 3:15 PM, Bruce Cherniak  
>> wrote:
>>> Clipper::ClipScalar() is dead code and should be removed.  It is causing
>>> an error with gcc-7 because it references a now defunct member.
>>>
>>> CC: "13.0 17.0" 
>>> ---
>>> src/gallium/drivers/swr/rasterizer/core/clip.h | 39 
>>> --
>>> 1 file changed, 39 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/swr/rasterizer/core/clip.h 
>>> b/src/gallium/drivers/swr/rasterizer/core/clip.h
>>> index 085e4a9..f19858f 100644
>>> --- a/src/gallium/drivers/swr/rasterizer/core/clip.h
>>> +++ b/src/gallium/drivers/swr/rasterizer/core/clip.h
>>> @@ -262,45 +262,6 @@ public:
>>> return _simd_movemask_ps(vClipCullMask);
>>> }
>>>
>>> -// clip a single primitive
>>> -int ClipScalar(PA_STATE& pa, uint32_t primIndex, float* pOutPos, 
>>> float* pOutAttribs)
>>> -{
>>> -OSALIGNSIMD(float) inVerts[3 * 4];
>>> -OSALIGNSIMD(float) inAttribs[3 * KNOB_NUM_ATTRIBUTES * 4];
>>> -
>>> -// transpose primitive position
>>> -__m128 verts[3];
>>> -pa.AssembleSingle(VERTEX_POSITION_SLOT, primIndex, verts);
>>> -_mm_store_ps([0], verts[0]);
>>> -_mm_store_ps([4], verts[1]);
>>> -_mm_store_ps([8], verts[2]);
>>> -
>>> -// transpose attribs
>>> -uint32_t numScalarAttribs = this->state.linkageCount * 4;
>>> -
>>> -int idx = 0;
>>> -DWORD slot = 0;
>>> -uint32_t mapIdx = 0;
>>> -uint32_t tmpLinkage = uint32_t(this->state.linkageMask);
>>> -while (_BitScanForward(, tmpLinkage))
>>> -{
>>> -tmpLinkage &= ~(1 << slot);
>>> -// Compute absolute attrib slot in vertex array
>>> -uint32_t inputSlot = VERTEX_ATTRIB_START_SLOT + 
>>> this->state.linkageMap[mapIdx++];
>>> -__m128 attrib[3];// triangle attribs (always 4 wide)
>>> -pa.AssembleSingle(inputSlot, primIndex, attrib);
>>> -_mm_store_ps([idx], attrib[0]);
>>> -_mm_store_ps([idx + numScalarAttribs], attrib[1]);
>>> -_mm_store_ps([idx + numScalarAttribs * 2], 
>>> attrib[2]);
>>> -idx += 4;
>>> -}
>>> -
>>> -int numVerts;
>>> -Clip(inVerts, inAttribs, numScalarAttribs, pOutPos, , 
>>> pOutAttribs);
>>> -
>>> -return numVerts;
>>> -}
>>> -
>>> // clip SIMD primitives
>>> void ClipSimd(const simdscalar& vPrimMask, const simdscalar& vClipMask, 
>>> PA_STATE& pa, const simdscalari& vPrimId, const simdscalari& vViewportIdx)
>>> {
>>> --
>>> 2.7.4
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] swr: [rasterizer core] Remove dead code Clipper::ClipScalar()

[Cherniak, Bruce]

I followed up with a v2 that includes the bugzilla reference.

Good point, I’ll look into following up with a patch to remove Clip().

Thanks for the quick review.

> On Feb 2, 2017, at 2:26 PM, Ilia Mirkin  wrote:
> 
> Reviewed-by: Ilia Mirkin 
> 
> I got confused by this code as well when I was trying to understand
> the clipper. I think the Clip() function can go too now in the .cpp
> file (as well as the fwd decl in the header)?
> 
> On Thu, Feb 2, 2017 at 3:15 PM, Bruce Cherniak  
> wrote:
>> Clipper::ClipScalar() is dead code and should be removed.  It is causing
>> an error with gcc-7 because it references a now defunct member.
>> 
>> CC: "13.0 17.0" 
>> ---
>> src/gallium/drivers/swr/rasterizer/core/clip.h | 39 
>> --
>> 1 file changed, 39 deletions(-)
>> 
>> diff --git a/src/gallium/drivers/swr/rasterizer/core/clip.h 
>> b/src/gallium/drivers/swr/rasterizer/core/clip.h
>> index 085e4a9..f19858f 100644
>> --- a/src/gallium/drivers/swr/rasterizer/core/clip.h
>> +++ b/src/gallium/drivers/swr/rasterizer/core/clip.h
>> @@ -262,45 +262,6 @@ public:
>> return _simd_movemask_ps(vClipCullMask);
>> }
>> 
>> -// clip a single primitive
>> -int ClipScalar(PA_STATE& pa, uint32_t primIndex, float* pOutPos, float* 
>> pOutAttribs)
>> -{
>> -OSALIGNSIMD(float) inVerts[3 * 4];
>> -OSALIGNSIMD(float) inAttribs[3 * KNOB_NUM_ATTRIBUTES * 4];
>> -
>> -// transpose primitive position
>> -__m128 verts[3];
>> -pa.AssembleSingle(VERTEX_POSITION_SLOT, primIndex, verts);
>> -_mm_store_ps([0], verts[0]);
>> -_mm_store_ps([4], verts[1]);
>> -_mm_store_ps([8], verts[2]);
>> -
>> -// transpose attribs
>> -uint32_t numScalarAttribs = this->state.linkageCount * 4;
>> -
>> -int idx = 0;
>> -DWORD slot = 0;
>> -uint32_t mapIdx = 0;
>> -uint32_t tmpLinkage = uint32_t(this->state.linkageMask);
>> -while (_BitScanForward(, tmpLinkage))
>> -{
>> -tmpLinkage &= ~(1 << slot);
>> -// Compute absolute attrib slot in vertex array
>> -uint32_t inputSlot = VERTEX_ATTRIB_START_SLOT + 
>> this->state.linkageMap[mapIdx++];
>> -__m128 attrib[3];// triangle attribs (always 4 wide)
>> -pa.AssembleSingle(inputSlot, primIndex, attrib);
>> -_mm_store_ps([idx], attrib[0]);
>> -_mm_store_ps([idx + numScalarAttribs], attrib[1]);
>> -_mm_store_ps([idx + numScalarAttribs * 2], attrib[2]);
>> -idx += 4;
>> -}
>> -
>> -int numVerts;
>> -Clip(inVerts, inAttribs, numScalarAttribs, pOutPos, , 
>> pOutAttribs);
>> -
>> -return numVerts;
>> -}
>> -
>> // clip SIMD primitives
>> void ClipSimd(const simdscalar& vPrimMask, const simdscalar& vClipMask, 
>> PA_STATE& pa, const simdscalari& vPrimId, const simdscalari& vViewportIdx)
>> {
>> --
>> 2.7.4
>> 
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] swr: [rasterizer core] Remove dead code Clipper::ClipScalar()

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 

I got confused by this code as well when I was trying to understand
the clipper. I think the Clip() function can go too now in the .cpp
file (as well as the fwd decl in the header)?

On Thu, Feb 2, 2017 at 3:15 PM, Bruce Cherniak  wrote:
> Clipper::ClipScalar() is dead code and should be removed.  It is causing
> an error with gcc-7 because it references a now defunct member.
>
> CC: "13.0 17.0" 
> ---
>  src/gallium/drivers/swr/rasterizer/core/clip.h | 39 
> --
>  1 file changed, 39 deletions(-)
>
> diff --git a/src/gallium/drivers/swr/rasterizer/core/clip.h 
> b/src/gallium/drivers/swr/rasterizer/core/clip.h
> index 085e4a9..f19858f 100644
> --- a/src/gallium/drivers/swr/rasterizer/core/clip.h
> +++ b/src/gallium/drivers/swr/rasterizer/core/clip.h
> @@ -262,45 +262,6 @@ public:
>  return _simd_movemask_ps(vClipCullMask);
>  }
>
> -// clip a single primitive
> -int ClipScalar(PA_STATE& pa, uint32_t primIndex, float* pOutPos, float* 
> pOutAttribs)
> -{
> -OSALIGNSIMD(float) inVerts[3 * 4];
> -OSALIGNSIMD(float) inAttribs[3 * KNOB_NUM_ATTRIBUTES * 4];
> -
> -// transpose primitive position
> -__m128 verts[3];
> -pa.AssembleSingle(VERTEX_POSITION_SLOT, primIndex, verts);
> -_mm_store_ps([0], verts[0]);
> -_mm_store_ps([4], verts[1]);
> -_mm_store_ps([8], verts[2]);
> -
> -// transpose attribs
> -uint32_t numScalarAttribs = this->state.linkageCount * 4;
> -
> -int idx = 0;
> -DWORD slot = 0;
> -uint32_t mapIdx = 0;
> -uint32_t tmpLinkage = uint32_t(this->state.linkageMask);
> -while (_BitScanForward(, tmpLinkage))
> -{
> -tmpLinkage &= ~(1 << slot);
> -// Compute absolute attrib slot in vertex array
> -uint32_t inputSlot = VERTEX_ATTRIB_START_SLOT + 
> this->state.linkageMap[mapIdx++];
> -__m128 attrib[3];// triangle attribs (always 4 wide)
> -pa.AssembleSingle(inputSlot, primIndex, attrib);
> -_mm_store_ps([idx], attrib[0]);
> -_mm_store_ps([idx + numScalarAttribs], attrib[1]);
> -_mm_store_ps([idx + numScalarAttribs * 2], attrib[2]);
> -idx += 4;
> -}
> -
> -int numVerts;
> -Clip(inVerts, inAttribs, numScalarAttribs, pOutPos, , 
> pOutAttribs);
> -
> -return numVerts;
> -}
> -
>  // clip SIMD primitives
>  void ClipSimd(const simdscalar& vPrimMask, const simdscalar& vClipMask, 
> PA_STATE& pa, const simdscalari& vPrimId, const simdscalari& vViewportIdx)
>  {
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] swr: [rasterizer core] Remove dead code Clipper::ClipScalar()

[Bruce Cherniak]

Clipper::ClipScalar() is dead code and should be removed.  It is causing
an error with gcc-7 because it references a now defunct member.

CC: "13.0 17.0" 
---
 src/gallium/drivers/swr/rasterizer/core/clip.h | 39 --
 1 file changed, 39 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/clip.h 
b/src/gallium/drivers/swr/rasterizer/core/clip.h
index 085e4a9..f19858f 100644
--- a/src/gallium/drivers/swr/rasterizer/core/clip.h
+++ b/src/gallium/drivers/swr/rasterizer/core/clip.h
@@ -262,45 +262,6 @@ public:
 return _simd_movemask_ps(vClipCullMask);
 }
 
-// clip a single primitive
-int ClipScalar(PA_STATE& pa, uint32_t primIndex, float* pOutPos, float* 
pOutAttribs)
-{
-OSALIGNSIMD(float) inVerts[3 * 4];
-OSALIGNSIMD(float) inAttribs[3 * KNOB_NUM_ATTRIBUTES * 4];
-
-// transpose primitive position
-__m128 verts[3];
-pa.AssembleSingle(VERTEX_POSITION_SLOT, primIndex, verts);
-_mm_store_ps([0], verts[0]);
-_mm_store_ps([4], verts[1]);
-_mm_store_ps([8], verts[2]);
-
-// transpose attribs
-uint32_t numScalarAttribs = this->state.linkageCount * 4;
-
-int idx = 0;
-DWORD slot = 0;
-uint32_t mapIdx = 0;
-uint32_t tmpLinkage = uint32_t(this->state.linkageMask);
-while (_BitScanForward(, tmpLinkage))
-{
-tmpLinkage &= ~(1 << slot);
-// Compute absolute attrib slot in vertex array
-uint32_t inputSlot = VERTEX_ATTRIB_START_SLOT + 
this->state.linkageMap[mapIdx++];
-__m128 attrib[3];// triangle attribs (always 4 wide)
-pa.AssembleSingle(inputSlot, primIndex, attrib);
-_mm_store_ps([idx], attrib[0]);
-_mm_store_ps([idx + numScalarAttribs], attrib[1]);
-_mm_store_ps([idx + numScalarAttribs * 2], attrib[2]);
-idx += 4;
-}
-
-int numVerts;
-Clip(inVerts, inAttribs, numScalarAttribs, pOutPos, , 
pOutAttribs);
-
-return numVerts;
-}
-
 // clip SIMD primitives
 void ClipSimd(const simdscalar& vPrimMask, const simdscalar& vClipMask, 
PA_STATE& pa, const simdscalari& vPrimId, const simdscalari& vViewportIdx)
 {
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev