Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Carter Schonwald
Oh ok.  The fix sounded like it was about core.  My mistake on reading your
explanation earlier !  Thanks for clarifying!

On Wed, Aug 1, 2018 at 11:45 PM Abhiroop Sarkar 
wrote:

> Oh no! it was a Cmm lint error. The register width don't come into the
> picture before Cmm. The register width in the Cmm is decided based on
> `primRepCmmType` function(which I mentioned in the previous mail) which in
> turn uses `Type`  and some other info, this is where the error was hiding.
> It was kind of hard to spot :)
>
> On Thu, Aug 2, 2018 at 4:38 AM Carter Schonwald <
> carter.schonw...@gmail.com> wrote:
>
>> So the issue was core lint type error rather than a cmm lint error?  Glad
>> you figured it out !
>>
>> But you didn’t communicate what the error you got from lint was core
>> rather than cmm ...  :(
>>
>> On Wed, Aug 1, 2018 at 11:25 PM Abhiroop Sarkar 
>> wrote:
>>
>>> Never mind I found the issue and fixed it.
>>>
>>> It was the definition of the `Int32` type constructor:
>>>
>>> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName IntRep
>>>
>>> which had to be fixed to
>>>
>>> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName Int32Rep
>>>
>>> Thanks anyways :)
>>>
>>> Abhiroop
>>>
>>> On Wed, Aug 1, 2018 at 5:36 PM Abhiroop Sarkar 
>>> wrote:
>>>
 Hello,

 I would appreciate some help in debugging a Cmm Lint error, I have been
 stuck on for quite a while.

 Basically I am adding support for Int32# on top of the In8#(D4475) and
 Int16#(D5006) patches.

 The Cmm being generated for the test programs are incorrect.

 Taking a sample test like this (
 https://github.com/Abhiroop/ghc-1/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt32.hs
 )

 The crux of the linting error is the type mismatch between LHS and RHS
 of the CmmAssign statement.

 For example in the Int8 patch, the Cmm generated looks like this:

 ```
_s1MC::I8 = I8[Sp];   // CmmAssign
_s1MD::I8 = I8[Sp + 8];   // CmmAssign
_s1ME::I8 = I8[Sp + 16];   // CmmAssign
_s1MF::I8 = I8[Sp + 24];   // CmmAssign
_s1MG::I8 = I8[Sp + 32];   // CmmAssign
_s1MH::I8 = I8[Sp + 40];   // CmmAssign
_s1MI::I8 = I8[Sp + 48];   // CmmAssign
_s1MJ::I8 = I8[Sp + 56];   // CmmAssign
_s1MK::I8 = I8[Sp + 64];   // CmmAssign
_s1ML::I8 = I8[Sp + 72];   // CmmAssign
R6 = %MO_XX_Conv_W8_W64(_s1MG::I8);   // CmmAssign
R5 = %MO_XX_Conv_W8_W64(_s1MF::I8);   // CmmAssign
R4 = %MO_XX_Conv_W8_W64(_s1ME::I8);   // CmmAssign
R3 = %MO_XX_Conv_W8_W64(_s1MD::I8);   // CmmAssign
R2 = %MO_XX_Conv_W8_W64(_s1MC::I8);   // CmmAssign
 ```

 or

 ```
_c1Oq::I8 = %MO_SS_Conv_W64_W8(9);   // CmmAssign
_s1N0::I8 = _c1Oq::I8;   // CmmAssign
_c1Ou::I8 = %MO_SS_Conv_W64_W8(8);   // CmmAssign
_s1MZ::I8 = _c1Ou::I8;   // CmmAssign
_c1Ox::I8 = %MO_SS_Conv_W64_W8(7);   // CmmAssign
_s1MY::I8 = _c1Ox::I8;   // CmmAssign
_c1OA::I8 = %MO_SS_Conv_W64_W8(6);   // CmmAssign
_s1MX::I8 = _c1OA::I8;   // CmmAssign
_c1OD::I8 = %MO_SS_Conv_W64_W8(5);   // CmmAssign
_s1MW::I8 = _c1OD::I8;   // CmmAssign
_c1OG::I8 = %MO_SS_Conv_W64_W8(4);   // CmmAssign
_s1MV::I8 = _c1OG::I8;   // CmmAssign
_c1OJ::I8 = %MO_SS_Conv_W64_W8(3);   // CmmAssign
_s1MU::I8 = _c1OJ::I8;   // CmmAssign
 ```

 Basically the registers on the left hand side have type `I8`, whereas
 in my case the registers on the LHS always have the width equal to the word
 size:

 ```
_c1Ok::I64 = %MO_SS_Conv_W64_W32(9);   // CmmAssign
_s1N0::I64 = _c1Ok::I64;   // CmmAssign
_c1Oo::I64 = %MO_SS_Conv_W64_W32(8);   // CmmAssign
_s1MZ::I64 = _c1Oo::I64;   // CmmAssign
_c1Or::I64 = %MO_SS_Conv_W64_W32(7);   // CmmAssign
_s1MY::I64 = _c1Or::I64;   // CmmAssign
_c1Ou::I64 = %MO_SS_Conv_W64_W32(6);   // CmmAssign
_s1MX::I64 = _c1Ou::I64;   // CmmAssign
_c1Ox::I64 = %MO_SS_Conv_W64_W32(5);   // CmmAssign
_s1MW::I64 = _c1Ox::I64;   // CmmAssign
_c1OA::I64 = %MO_SS_Conv_W64_W32(4);   // CmmAssign
_s1MV::I64 = _c1OA::I64;   // CmmAssign
_c1OD::I64 = %MO_SS_Conv_W64_W32(3);   // CmmAssign
_s1MU::I64 = _c1OD::I64;   // CmmAssign
_c1OG::I64 = %MO_SS_Conv_W64_W32(2);   // CmmAssign
_s1MT::I64 = _c1OG::I64;   // CmmAssign
 ```
 I would ideally want to have the datatype of the LHS be `I32`.

 Michal, I thought this type is picked 

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Abhiroop Sarkar
Oh no! it was a Cmm lint error. The register width don't come into the
picture before Cmm. The register width in the Cmm is decided based on
`primRepCmmType` function(which I mentioned in the previous mail) which in
turn uses `Type`  and some other info, this is where the error was hiding.
It was kind of hard to spot :)

On Thu, Aug 2, 2018 at 4:38 AM Carter Schonwald 
wrote:

> So the issue was core lint type error rather than a cmm lint error?  Glad
> you figured it out !
>
> But you didn’t communicate what the error you got from lint was core
> rather than cmm ...  :(
>
> On Wed, Aug 1, 2018 at 11:25 PM Abhiroop Sarkar 
> wrote:
>
>> Never mind I found the issue and fixed it.
>>
>> It was the definition of the `Int32` type constructor:
>>
>> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName IntRep
>>
>> which had to be fixed to
>>
>> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName Int32Rep
>>
>> Thanks anyways :)
>>
>> Abhiroop
>>
>> On Wed, Aug 1, 2018 at 5:36 PM Abhiroop Sarkar 
>> wrote:
>>
>>> Hello,
>>>
>>> I would appreciate some help in debugging a Cmm Lint error, I have been
>>> stuck on for quite a while.
>>>
>>> Basically I am adding support for Int32# on top of the In8#(D4475) and
>>> Int16#(D5006) patches.
>>>
>>> The Cmm being generated for the test programs are incorrect.
>>>
>>> Taking a sample test like this (
>>> https://github.com/Abhiroop/ghc-1/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt32.hs
>>> )
>>>
>>> The crux of the linting error is the type mismatch between LHS and RHS
>>> of the CmmAssign statement.
>>>
>>> For example in the Int8 patch, the Cmm generated looks like this:
>>>
>>> ```
>>>_s1MC::I8 = I8[Sp];   // CmmAssign
>>>_s1MD::I8 = I8[Sp + 8];   // CmmAssign
>>>_s1ME::I8 = I8[Sp + 16];   // CmmAssign
>>>_s1MF::I8 = I8[Sp + 24];   // CmmAssign
>>>_s1MG::I8 = I8[Sp + 32];   // CmmAssign
>>>_s1MH::I8 = I8[Sp + 40];   // CmmAssign
>>>_s1MI::I8 = I8[Sp + 48];   // CmmAssign
>>>_s1MJ::I8 = I8[Sp + 56];   // CmmAssign
>>>_s1MK::I8 = I8[Sp + 64];   // CmmAssign
>>>_s1ML::I8 = I8[Sp + 72];   // CmmAssign
>>>R6 = %MO_XX_Conv_W8_W64(_s1MG::I8);   // CmmAssign
>>>R5 = %MO_XX_Conv_W8_W64(_s1MF::I8);   // CmmAssign
>>>R4 = %MO_XX_Conv_W8_W64(_s1ME::I8);   // CmmAssign
>>>R3 = %MO_XX_Conv_W8_W64(_s1MD::I8);   // CmmAssign
>>>R2 = %MO_XX_Conv_W8_W64(_s1MC::I8);   // CmmAssign
>>> ```
>>>
>>> or
>>>
>>> ```
>>>_c1Oq::I8 = %MO_SS_Conv_W64_W8(9);   // CmmAssign
>>>_s1N0::I8 = _c1Oq::I8;   // CmmAssign
>>>_c1Ou::I8 = %MO_SS_Conv_W64_W8(8);   // CmmAssign
>>>_s1MZ::I8 = _c1Ou::I8;   // CmmAssign
>>>_c1Ox::I8 = %MO_SS_Conv_W64_W8(7);   // CmmAssign
>>>_s1MY::I8 = _c1Ox::I8;   // CmmAssign
>>>_c1OA::I8 = %MO_SS_Conv_W64_W8(6);   // CmmAssign
>>>_s1MX::I8 = _c1OA::I8;   // CmmAssign
>>>_c1OD::I8 = %MO_SS_Conv_W64_W8(5);   // CmmAssign
>>>_s1MW::I8 = _c1OD::I8;   // CmmAssign
>>>_c1OG::I8 = %MO_SS_Conv_W64_W8(4);   // CmmAssign
>>>_s1MV::I8 = _c1OG::I8;   // CmmAssign
>>>_c1OJ::I8 = %MO_SS_Conv_W64_W8(3);   // CmmAssign
>>>_s1MU::I8 = _c1OJ::I8;   // CmmAssign
>>> ```
>>>
>>> Basically the registers on the left hand side have type `I8`, whereas in
>>> my case the registers on the LHS always have the width equal to the word
>>> size:
>>>
>>> ```
>>>_c1Ok::I64 = %MO_SS_Conv_W64_W32(9);   // CmmAssign
>>>_s1N0::I64 = _c1Ok::I64;   // CmmAssign
>>>_c1Oo::I64 = %MO_SS_Conv_W64_W32(8);   // CmmAssign
>>>_s1MZ::I64 = _c1Oo::I64;   // CmmAssign
>>>_c1Or::I64 = %MO_SS_Conv_W64_W32(7);   // CmmAssign
>>>_s1MY::I64 = _c1Or::I64;   // CmmAssign
>>>_c1Ou::I64 = %MO_SS_Conv_W64_W32(6);   // CmmAssign
>>>_s1MX::I64 = _c1Ou::I64;   // CmmAssign
>>>_c1Ox::I64 = %MO_SS_Conv_W64_W32(5);   // CmmAssign
>>>_s1MW::I64 = _c1Ox::I64;   // CmmAssign
>>>_c1OA::I64 = %MO_SS_Conv_W64_W32(4);   // CmmAssign
>>>_s1MV::I64 = _c1OA::I64;   // CmmAssign
>>>_c1OD::I64 = %MO_SS_Conv_W64_W32(3);   // CmmAssign
>>>_s1MU::I64 = _c1OD::I64;   // CmmAssign
>>>_c1OG::I64 = %MO_SS_Conv_W64_W32(2);   // CmmAssign
>>>_s1MT::I64 = _c1OG::I64;   // CmmAssign
>>> ```
>>> I would ideally want to have the datatype of the LHS be `I32`.
>>>
>>> Michal, I thought this type is picked up using the `primRepCmmType`
>>> function
>>> https://github.com/Abhiroop/ghc-1/blob/int8/compiler/cmm/CmmUtils.hs#L104-L105
>>>  which
>>> I modified but I am not sure why the LHS types of the CmmAssign statement
>>> allocate `I64` instead of `I32`. Is there any other change on your
>>> patch(D4475) which I might have 

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Carter Schonwald
So the issue was core lint type error rather than a cmm lint error?  Glad
you figured it out !

But you didn’t communicate what the error you got from lint was core rather
than cmm ...  :(

On Wed, Aug 1, 2018 at 11:25 PM Abhiroop Sarkar 
wrote:

> Never mind I found the issue and fixed it.
>
> It was the definition of the `Int32` type constructor:
>
> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName IntRep
>
> which had to be fixed to
>
> int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName Int32Rep
>
> Thanks anyways :)
>
> Abhiroop
>
> On Wed, Aug 1, 2018 at 5:36 PM Abhiroop Sarkar 
> wrote:
>
>> Hello,
>>
>> I would appreciate some help in debugging a Cmm Lint error, I have been
>> stuck on for quite a while.
>>
>> Basically I am adding support for Int32# on top of the In8#(D4475) and
>> Int16#(D5006) patches.
>>
>> The Cmm being generated for the test programs are incorrect.
>>
>> Taking a sample test like this (
>> https://github.com/Abhiroop/ghc-1/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt32.hs
>> )
>>
>> The crux of the linting error is the type mismatch between LHS and RHS of
>> the CmmAssign statement.
>>
>> For example in the Int8 patch, the Cmm generated looks like this:
>>
>> ```
>>_s1MC::I8 = I8[Sp];   // CmmAssign
>>_s1MD::I8 = I8[Sp + 8];   // CmmAssign
>>_s1ME::I8 = I8[Sp + 16];   // CmmAssign
>>_s1MF::I8 = I8[Sp + 24];   // CmmAssign
>>_s1MG::I8 = I8[Sp + 32];   // CmmAssign
>>_s1MH::I8 = I8[Sp + 40];   // CmmAssign
>>_s1MI::I8 = I8[Sp + 48];   // CmmAssign
>>_s1MJ::I8 = I8[Sp + 56];   // CmmAssign
>>_s1MK::I8 = I8[Sp + 64];   // CmmAssign
>>_s1ML::I8 = I8[Sp + 72];   // CmmAssign
>>R6 = %MO_XX_Conv_W8_W64(_s1MG::I8);   // CmmAssign
>>R5 = %MO_XX_Conv_W8_W64(_s1MF::I8);   // CmmAssign
>>R4 = %MO_XX_Conv_W8_W64(_s1ME::I8);   // CmmAssign
>>R3 = %MO_XX_Conv_W8_W64(_s1MD::I8);   // CmmAssign
>>R2 = %MO_XX_Conv_W8_W64(_s1MC::I8);   // CmmAssign
>> ```
>>
>> or
>>
>> ```
>>_c1Oq::I8 = %MO_SS_Conv_W64_W8(9);   // CmmAssign
>>_s1N0::I8 = _c1Oq::I8;   // CmmAssign
>>_c1Ou::I8 = %MO_SS_Conv_W64_W8(8);   // CmmAssign
>>_s1MZ::I8 = _c1Ou::I8;   // CmmAssign
>>_c1Ox::I8 = %MO_SS_Conv_W64_W8(7);   // CmmAssign
>>_s1MY::I8 = _c1Ox::I8;   // CmmAssign
>>_c1OA::I8 = %MO_SS_Conv_W64_W8(6);   // CmmAssign
>>_s1MX::I8 = _c1OA::I8;   // CmmAssign
>>_c1OD::I8 = %MO_SS_Conv_W64_W8(5);   // CmmAssign
>>_s1MW::I8 = _c1OD::I8;   // CmmAssign
>>_c1OG::I8 = %MO_SS_Conv_W64_W8(4);   // CmmAssign
>>_s1MV::I8 = _c1OG::I8;   // CmmAssign
>>_c1OJ::I8 = %MO_SS_Conv_W64_W8(3);   // CmmAssign
>>_s1MU::I8 = _c1OJ::I8;   // CmmAssign
>> ```
>>
>> Basically the registers on the left hand side have type `I8`, whereas in
>> my case the registers on the LHS always have the width equal to the word
>> size:
>>
>> ```
>>_c1Ok::I64 = %MO_SS_Conv_W64_W32(9);   // CmmAssign
>>_s1N0::I64 = _c1Ok::I64;   // CmmAssign
>>_c1Oo::I64 = %MO_SS_Conv_W64_W32(8);   // CmmAssign
>>_s1MZ::I64 = _c1Oo::I64;   // CmmAssign
>>_c1Or::I64 = %MO_SS_Conv_W64_W32(7);   // CmmAssign
>>_s1MY::I64 = _c1Or::I64;   // CmmAssign
>>_c1Ou::I64 = %MO_SS_Conv_W64_W32(6);   // CmmAssign
>>_s1MX::I64 = _c1Ou::I64;   // CmmAssign
>>_c1Ox::I64 = %MO_SS_Conv_W64_W32(5);   // CmmAssign
>>_s1MW::I64 = _c1Ox::I64;   // CmmAssign
>>_c1OA::I64 = %MO_SS_Conv_W64_W32(4);   // CmmAssign
>>_s1MV::I64 = _c1OA::I64;   // CmmAssign
>>_c1OD::I64 = %MO_SS_Conv_W64_W32(3);   // CmmAssign
>>_s1MU::I64 = _c1OD::I64;   // CmmAssign
>>_c1OG::I64 = %MO_SS_Conv_W64_W32(2);   // CmmAssign
>>_s1MT::I64 = _c1OG::I64;   // CmmAssign
>> ```
>> I would ideally want to have the datatype of the LHS be `I32`.
>>
>> Michal, I thought this type is picked up using the `primRepCmmType`
>> function
>> https://github.com/Abhiroop/ghc-1/blob/int8/compiler/cmm/CmmUtils.hs#L104-L105
>>  which
>> I modified but I am not sure why the LHS types of the CmmAssign statement
>> allocate `I64` instead of `I32`. Is there any other change on your
>> patch(D4475) which I might have overlooked.
>>
>> I have uploaded my Int32# patch on phab as well for reference(
>> https://phabricator.haskell.org/D5032)
>>
>> Thanks,
>> Abhiroop
>>
>>
>>
>> On Wed, Aug 1, 2018 at 3:45 PM Michal Terepeta 
>> wrote:
>>
>>> I've rebased the diff and relaxed the assertion - do take a look if that
>>> looks reasonable to you :)
>>> https://phabricator.haskell.org/D4475
>>>
>>> Cheers!
>>>
>>> - Michal
>>>
>>> On Wed, Jul 25, 2018 at 9:03 PM Michal Terepeta <
>>> michal.terep...@gmail.com> wrote:
>>>

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Abhiroop Sarkar
Never mind I found the issue and fixed it.

It was the definition of the `Int32` type constructor:

int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName IntRep

which had to be fixed to

int32PrimTyCon  = pcPrimTyCon0 int32PrimTyConName Int32Rep

Thanks anyways :)

Abhiroop

On Wed, Aug 1, 2018 at 5:36 PM Abhiroop Sarkar 
wrote:

> Hello,
>
> I would appreciate some help in debugging a Cmm Lint error, I have been
> stuck on for quite a while.
>
> Basically I am adding support for Int32# on top of the In8#(D4475) and
> Int16#(D5006) patches.
>
> The Cmm being generated for the test programs are incorrect.
>
> Taking a sample test like this (
> https://github.com/Abhiroop/ghc-1/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt32.hs
> )
>
> The crux of the linting error is the type mismatch between LHS and RHS of
> the CmmAssign statement.
>
> For example in the Int8 patch, the Cmm generated looks like this:
>
> ```
>_s1MC::I8 = I8[Sp];   // CmmAssign
>_s1MD::I8 = I8[Sp + 8];   // CmmAssign
>_s1ME::I8 = I8[Sp + 16];   // CmmAssign
>_s1MF::I8 = I8[Sp + 24];   // CmmAssign
>_s1MG::I8 = I8[Sp + 32];   // CmmAssign
>_s1MH::I8 = I8[Sp + 40];   // CmmAssign
>_s1MI::I8 = I8[Sp + 48];   // CmmAssign
>_s1MJ::I8 = I8[Sp + 56];   // CmmAssign
>_s1MK::I8 = I8[Sp + 64];   // CmmAssign
>_s1ML::I8 = I8[Sp + 72];   // CmmAssign
>R6 = %MO_XX_Conv_W8_W64(_s1MG::I8);   // CmmAssign
>R5 = %MO_XX_Conv_W8_W64(_s1MF::I8);   // CmmAssign
>R4 = %MO_XX_Conv_W8_W64(_s1ME::I8);   // CmmAssign
>R3 = %MO_XX_Conv_W8_W64(_s1MD::I8);   // CmmAssign
>R2 = %MO_XX_Conv_W8_W64(_s1MC::I8);   // CmmAssign
> ```
>
> or
>
> ```
>_c1Oq::I8 = %MO_SS_Conv_W64_W8(9);   // CmmAssign
>_s1N0::I8 = _c1Oq::I8;   // CmmAssign
>_c1Ou::I8 = %MO_SS_Conv_W64_W8(8);   // CmmAssign
>_s1MZ::I8 = _c1Ou::I8;   // CmmAssign
>_c1Ox::I8 = %MO_SS_Conv_W64_W8(7);   // CmmAssign
>_s1MY::I8 = _c1Ox::I8;   // CmmAssign
>_c1OA::I8 = %MO_SS_Conv_W64_W8(6);   // CmmAssign
>_s1MX::I8 = _c1OA::I8;   // CmmAssign
>_c1OD::I8 = %MO_SS_Conv_W64_W8(5);   // CmmAssign
>_s1MW::I8 = _c1OD::I8;   // CmmAssign
>_c1OG::I8 = %MO_SS_Conv_W64_W8(4);   // CmmAssign
>_s1MV::I8 = _c1OG::I8;   // CmmAssign
>_c1OJ::I8 = %MO_SS_Conv_W64_W8(3);   // CmmAssign
>_s1MU::I8 = _c1OJ::I8;   // CmmAssign
> ```
>
> Basically the registers on the left hand side have type `I8`, whereas in
> my case the registers on the LHS always have the width equal to the word
> size:
>
> ```
>_c1Ok::I64 = %MO_SS_Conv_W64_W32(9);   // CmmAssign
>_s1N0::I64 = _c1Ok::I64;   // CmmAssign
>_c1Oo::I64 = %MO_SS_Conv_W64_W32(8);   // CmmAssign
>_s1MZ::I64 = _c1Oo::I64;   // CmmAssign
>_c1Or::I64 = %MO_SS_Conv_W64_W32(7);   // CmmAssign
>_s1MY::I64 = _c1Or::I64;   // CmmAssign
>_c1Ou::I64 = %MO_SS_Conv_W64_W32(6);   // CmmAssign
>_s1MX::I64 = _c1Ou::I64;   // CmmAssign
>_c1Ox::I64 = %MO_SS_Conv_W64_W32(5);   // CmmAssign
>_s1MW::I64 = _c1Ox::I64;   // CmmAssign
>_c1OA::I64 = %MO_SS_Conv_W64_W32(4);   // CmmAssign
>_s1MV::I64 = _c1OA::I64;   // CmmAssign
>_c1OD::I64 = %MO_SS_Conv_W64_W32(3);   // CmmAssign
>_s1MU::I64 = _c1OD::I64;   // CmmAssign
>_c1OG::I64 = %MO_SS_Conv_W64_W32(2);   // CmmAssign
>_s1MT::I64 = _c1OG::I64;   // CmmAssign
> ```
> I would ideally want to have the datatype of the LHS be `I32`.
>
> Michal, I thought this type is picked up using the `primRepCmmType`
> function
> https://github.com/Abhiroop/ghc-1/blob/int8/compiler/cmm/CmmUtils.hs#L104-L105
>  which
> I modified but I am not sure why the LHS types of the CmmAssign statement
> allocate `I64` instead of `I32`. Is there any other change on your
> patch(D4475) which I might have overlooked.
>
> I have uploaded my Int32# patch on phab as well for reference(
> https://phabricator.haskell.org/D5032)
>
> Thanks,
> Abhiroop
>
>
>
> On Wed, Aug 1, 2018 at 3:45 PM Michal Terepeta 
> wrote:
>
>> I've rebased the diff and relaxed the assertion - do take a look if that
>> looks reasonable to you :)
>> https://phabricator.haskell.org/D4475
>>
>> Cheers!
>>
>> - Michal
>>
>> On Wed, Jul 25, 2018 at 9:03 PM Michal Terepeta <
>> michal.terep...@gmail.com> wrote:
>>
>>> Hi Carter,
>>>
>>> I didn't write this assertion. I only validated locally (IIRC at the
>>> time I uploaded the diff, harbormaster was failing for some other reasons).
>>>
>>> I'll try to have a look at it this weekend.
>>>
>>> Cheers!
>>>
>>> - Michal
>>>
>>> On Wed, Jul 25, 2018 at 2:16 AM Carter Schonwald <
>>> carter.schonw...@gmail.com> wrote:
>>>
 Michal: did you write 

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Abhiroop Sarkar
Hello,

I would appreciate some help in debugging a Cmm Lint error, I have been
stuck on for quite a while.

Basically I am adding support for Int32# on top of the In8#(D4475) and
Int16#(D5006) patches.

The Cmm being generated for the test programs are incorrect.

Taking a sample test like this (
https://github.com/Abhiroop/ghc-1/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt32.hs
)

The crux of the linting error is the type mismatch between LHS and RHS of
the CmmAssign statement.

For example in the Int8 patch, the Cmm generated looks like this:

```
   _s1MC::I8 = I8[Sp];   // CmmAssign
   _s1MD::I8 = I8[Sp + 8];   // CmmAssign
   _s1ME::I8 = I8[Sp + 16];   // CmmAssign
   _s1MF::I8 = I8[Sp + 24];   // CmmAssign
   _s1MG::I8 = I8[Sp + 32];   // CmmAssign
   _s1MH::I8 = I8[Sp + 40];   // CmmAssign
   _s1MI::I8 = I8[Sp + 48];   // CmmAssign
   _s1MJ::I8 = I8[Sp + 56];   // CmmAssign
   _s1MK::I8 = I8[Sp + 64];   // CmmAssign
   _s1ML::I8 = I8[Sp + 72];   // CmmAssign
   R6 = %MO_XX_Conv_W8_W64(_s1MG::I8);   // CmmAssign
   R5 = %MO_XX_Conv_W8_W64(_s1MF::I8);   // CmmAssign
   R4 = %MO_XX_Conv_W8_W64(_s1ME::I8);   // CmmAssign
   R3 = %MO_XX_Conv_W8_W64(_s1MD::I8);   // CmmAssign
   R2 = %MO_XX_Conv_W8_W64(_s1MC::I8);   // CmmAssign
```

or

```
   _c1Oq::I8 = %MO_SS_Conv_W64_W8(9);   // CmmAssign
   _s1N0::I8 = _c1Oq::I8;   // CmmAssign
   _c1Ou::I8 = %MO_SS_Conv_W64_W8(8);   // CmmAssign
   _s1MZ::I8 = _c1Ou::I8;   // CmmAssign
   _c1Ox::I8 = %MO_SS_Conv_W64_W8(7);   // CmmAssign
   _s1MY::I8 = _c1Ox::I8;   // CmmAssign
   _c1OA::I8 = %MO_SS_Conv_W64_W8(6);   // CmmAssign
   _s1MX::I8 = _c1OA::I8;   // CmmAssign
   _c1OD::I8 = %MO_SS_Conv_W64_W8(5);   // CmmAssign
   _s1MW::I8 = _c1OD::I8;   // CmmAssign
   _c1OG::I8 = %MO_SS_Conv_W64_W8(4);   // CmmAssign
   _s1MV::I8 = _c1OG::I8;   // CmmAssign
   _c1OJ::I8 = %MO_SS_Conv_W64_W8(3);   // CmmAssign
   _s1MU::I8 = _c1OJ::I8;   // CmmAssign
```

Basically the registers on the left hand side have type `I8`, whereas in my
case the registers on the LHS always have the width equal to the word size:

```
   _c1Ok::I64 = %MO_SS_Conv_W64_W32(9);   // CmmAssign
   _s1N0::I64 = _c1Ok::I64;   // CmmAssign
   _c1Oo::I64 = %MO_SS_Conv_W64_W32(8);   // CmmAssign
   _s1MZ::I64 = _c1Oo::I64;   // CmmAssign
   _c1Or::I64 = %MO_SS_Conv_W64_W32(7);   // CmmAssign
   _s1MY::I64 = _c1Or::I64;   // CmmAssign
   _c1Ou::I64 = %MO_SS_Conv_W64_W32(6);   // CmmAssign
   _s1MX::I64 = _c1Ou::I64;   // CmmAssign
   _c1Ox::I64 = %MO_SS_Conv_W64_W32(5);   // CmmAssign
   _s1MW::I64 = _c1Ox::I64;   // CmmAssign
   _c1OA::I64 = %MO_SS_Conv_W64_W32(4);   // CmmAssign
   _s1MV::I64 = _c1OA::I64;   // CmmAssign
   _c1OD::I64 = %MO_SS_Conv_W64_W32(3);   // CmmAssign
   _s1MU::I64 = _c1OD::I64;   // CmmAssign
   _c1OG::I64 = %MO_SS_Conv_W64_W32(2);   // CmmAssign
   _s1MT::I64 = _c1OG::I64;   // CmmAssign
```
I would ideally want to have the datatype of the LHS be `I32`.

Michal, I thought this type is picked up using the `primRepCmmType`
function
https://github.com/Abhiroop/ghc-1/blob/int8/compiler/cmm/CmmUtils.hs#L104-L105
which
I modified but I am not sure why the LHS types of the CmmAssign statement
allocate `I64` instead of `I32`. Is there any other change on your
patch(D4475) which I might have overlooked.

I have uploaded my Int32# patch on phab as well for reference(
https://phabricator.haskell.org/D5032)

Thanks,
Abhiroop



On Wed, Aug 1, 2018 at 3:45 PM Michal Terepeta 
wrote:

> I've rebased the diff and relaxed the assertion - do take a look if that
> looks reasonable to you :)
> https://phabricator.haskell.org/D4475
>
> Cheers!
>
> - Michal
>
> On Wed, Jul 25, 2018 at 9:03 PM Michal Terepeta 
> wrote:
>
>> Hi Carter,
>>
>> I didn't write this assertion. I only validated locally (IIRC at the time
>> I uploaded the diff, harbormaster was failing for some other reasons).
>>
>> I'll try to have a look at it this weekend.
>>
>> Cheers!
>>
>> - Michal
>>
>> On Wed, Jul 25, 2018 at 2:16 AM Carter Schonwald <
>> carter.schonw...@gmail.com> wrote:
>>
>>> Michal: did you write this Assert about width? and if so could you
>>> explain it so we can understand?
>>>
>>> hrmm... that code is in the procedure for generating C calls for 64bit
>>> intel systems
>>>
>>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2541
>>> is the  top of that routine
>>>
>>> and width is defined right below the spot in question
>>>
>>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764-L2774
>>>
>>> it seems like the code/assertion likely predates michal's work? (eg, a
>>> trick to make sure that 

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-08-01 Thread Michal Terepeta
I've rebased the diff and relaxed the assertion - do take a look if that
looks reasonable to you :)
https://phabricator.haskell.org/D4475

Cheers!

- Michal

On Wed, Jul 25, 2018 at 9:03 PM Michal Terepeta 
wrote:

> Hi Carter,
>
> I didn't write this assertion. I only validated locally (IIRC at the time
> I uploaded the diff, harbormaster was failing for some other reasons).
>
> I'll try to have a look at it this weekend.
>
> Cheers!
>
> - Michal
>
> On Wed, Jul 25, 2018 at 2:16 AM Carter Schonwald <
> carter.schonw...@gmail.com> wrote:
>
>> Michal: did you write this Assert about width? and if so could you
>> explain it so we can understand?
>>
>> hrmm... that code is in the procedure for generating C calls for 64bit
>> intel systems
>>
>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2541
>> is the  top of that routine
>>
>> and width is defined right below the spot in question
>>
>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764-L2774
>>
>> it seems like the code/assertion likely predates michal's work? (eg, a
>> trick to make sure that genCCall64 doesn't get invoked by 32bit platforms?)
>>
>> On Mon, Jul 23, 2018 at 8:54 PM Abhiroop Sarkar 
>> wrote:
>>
>>> Hi Michal,
>>>
>>> In the tests that you have added to D4475, are all the tests running
>>> fine?
>>>
>>> On my machine, I was running the FFI tests(
>>> https://github.com/michalt/ghc/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt8.hs)
>>> and they seem to fail at a particular assert statement in the code
>>> generator.
>>>
>>> To be precise this one:
>>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764
>>>
>>> Upon commenting that assert the tests run fine. Am I missing something
>>> or is the failure expected?
>>>
>>> Thanks,
>>> Abhiroop
>>>
>>> On Mon, Jul 9, 2018 at 8:31 PM Michal Terepeta <
>>> michal.terep...@gmail.com> wrote:
>>>
 Just for the record, I've uploaded the changes to binary:
 https://github.com/michalt/packages-binary/tree/int8

 - Michal

 On Wed, Jul 4, 2018 at 11:07 AM Michal Terepeta <
 michal.terep...@gmail.com> wrote:

> Yeah, if you look at the linked diff, there are a few tiny changes to
> binary that are necessary.
>
> I'll try to upload them to github later this week.
>
> Ben, is something blocking the review of the diff? I think I addressed
> all comments so far.
>
> - Michal
>
> On Wed, Jul 4, 2018 at 1:38 AM Abhiroop Sarkar 
> wrote:
>
>> Hello Michal,
>>
>> I was looking at your diff https://phabricator.haskell.org/D4475
>> and there seems to be some changes that you perhaps made in the binary
>> package (
>> https://phabricator.haskell.org/differential/changeset/?ref=199152=ignore-most).
>> I could not find your version of binary on your github repos list. Is it
>> possible for you to upload that so I can pull those changes?
>>
>> Thanks
>>
>> Abhiroop
>>
>> On Mon, May 28, 2018 at 10:45 PM Carter Schonwald <
>> carter.schonw...@gmail.com> wrote:
>>
>>> Abhiroop has the gist, though the size of word args for simd is more
>>> something we want to be consistent between 32/64 bit modes aka ghc
>>> targeting 32 or 64 bit intel with simd support :).  It would be best if 
>>> the
>>> unpack and pack operations that map to and from unboxed tuples where
>>> consistent and precise type wise.
>>>
>>>
>>>
>>> -Carter
>>>
>>> On May 28, 2018, at 5:02 PM, Abhiroop Sarkar 
>>> wrote:
>>>
>>> Hello Michal,
>>>
>>> My understanding of the issues are much lesser compared to Carter.
>>> However, I will state whatever I understood from discussions with him. 
>>> Be
>>> warned my understanding might be wrong and Carter might be asking this 
>>> for
>>> some completely different reason.
>>>
>>> > Out of curiosity, why do you need Word64#/Word32# story to be
>>> fixed for SIMD?
>>>
>>> One of the issues we are dealing with is multiple
>>> microarchitectures(SSE, AVX, AVX2 etc). As a result different
>>> microarchitectures has different ways of handling an 8 bit or 16 bit or 
>>> 32
>>> bit unsigned integer. An example I can provide is the vector multiply
>>> instruction which has different semantics of multiplying and storing the
>>> first 16 bits of a 32 bit unsigned integer compared to the lower 16 bits
>>> for each of the elements in its SIMD registers. There will be some other
>>> intricacies around handling a byte sized integer or a 64 bit integer 
>>> which
>>> will be different from the 32 bit version.
>>>
>>> Basically a 128 bit vector instruction will make some minor
>>> differences when dealing with 2 64 bit integers or 4 32 bit integer or 
>>> 8 16
>>> bit integers.
>>>
>>> So I think at the higher level 

Re: understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-07-25 Thread Michal Terepeta
Hi Carter,

I didn't write this assertion. I only validated locally (IIRC at the time I
uploaded the diff, harbormaster was failing for some other reasons).

I'll try to have a look at it this weekend.

Cheers!

- Michal

On Wed, Jul 25, 2018 at 2:16 AM Carter Schonwald 
wrote:

> Michal: did you write this Assert about width? and if so could you explain
> it so we can understand?
>
> hrmm... that code is in the procedure for generating C calls for 64bit
> intel systems
>
> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2541
> is the  top of that routine
>
> and width is defined right below the spot in question
>
> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764-L2774
>
> it seems like the code/assertion likely predates michal's work? (eg, a
> trick to make sure that genCCall64 doesn't get invoked by 32bit platforms?)
>
> On Mon, Jul 23, 2018 at 8:54 PM Abhiroop Sarkar 
> wrote:
>
>> Hi Michal,
>>
>> In the tests that you have added to D4475, are all the tests running fine?
>>
>> On my machine, I was running the FFI tests(
>> https://github.com/michalt/ghc/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt8.hs)
>> and they seem to fail at a particular assert statement in the code
>> generator.
>>
>> To be precise this one:
>> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764
>>
>> Upon commenting that assert the tests run fine. Am I missing something or
>> is the failure expected?
>>
>> Thanks,
>> Abhiroop
>>
>> On Mon, Jul 9, 2018 at 8:31 PM Michal Terepeta 
>> wrote:
>>
>>> Just for the record, I've uploaded the changes to binary:
>>> https://github.com/michalt/packages-binary/tree/int8
>>>
>>> - Michal
>>>
>>> On Wed, Jul 4, 2018 at 11:07 AM Michal Terepeta <
>>> michal.terep...@gmail.com> wrote:
>>>
 Yeah, if you look at the linked diff, there are a few tiny changes to
 binary that are necessary.

 I'll try to upload them to github later this week.

 Ben, is something blocking the review of the diff? I think I addressed
 all comments so far.

 - Michal

 On Wed, Jul 4, 2018 at 1:38 AM Abhiroop Sarkar 
 wrote:

> Hello Michal,
>
> I was looking at your diff https://phabricator.haskell.org/D4475  and
> there seems to be some changes that you perhaps made in the binary 
> package (
> https://phabricator.haskell.org/differential/changeset/?ref=199152=ignore-most).
> I could not find your version of binary on your github repos list. Is it
> possible for you to upload that so I can pull those changes?
>
> Thanks
>
> Abhiroop
>
> On Mon, May 28, 2018 at 10:45 PM Carter Schonwald <
> carter.schonw...@gmail.com> wrote:
>
>> Abhiroop has the gist, though the size of word args for simd is more
>> something we want to be consistent between 32/64 bit modes aka ghc
>> targeting 32 or 64 bit intel with simd support :).  It would be best if 
>> the
>> unpack and pack operations that map to and from unboxed tuples where
>> consistent and precise type wise.
>>
>>
>>
>> -Carter
>>
>> On May 28, 2018, at 5:02 PM, Abhiroop Sarkar 
>> wrote:
>>
>> Hello Michal,
>>
>> My understanding of the issues are much lesser compared to Carter.
>> However, I will state whatever I understood from discussions with him. Be
>> warned my understanding might be wrong and Carter might be asking this 
>> for
>> some completely different reason.
>>
>> > Out of curiosity, why do you need Word64#/Word32# story to be fixed
>> for SIMD?
>>
>> One of the issues we are dealing with is multiple
>> microarchitectures(SSE, AVX, AVX2 etc). As a result different
>> microarchitectures has different ways of handling an 8 bit or 16 bit or 
>> 32
>> bit unsigned integer. An example I can provide is the vector multiply
>> instruction which has different semantics of multiplying and storing the
>> first 16 bits of a 32 bit unsigned integer compared to the lower 16 bits
>> for each of the elements in its SIMD registers. There will be some other
>> intricacies around handling a byte sized integer or a 64 bit integer 
>> which
>> will be different from the 32 bit version.
>>
>> Basically a 128 bit vector instruction will make some minor
>> differences when dealing with 2 64 bit integers or 4 32 bit integer or 8 
>> 16
>> bit integers.
>>
>> So I think at the higher level we would want to be as precise as
>> possible when specifying the datatypes and want things like Word8#,
>> Word16#, Word32#, Word64# rather than a single ambiguous type like Word.
>>
>> One more example is this vector operation like : broadcastWord64X2#
>> :: Word#
>> 
>>  -> Word64X2#
>> 

understanding assertions, part deux :) Re: whither built in unlifted Word32# / Word64# etc?

2018-07-24 Thread Carter Schonwald
Michal: did you write this Assert about width? and if so could you explain
it so we can understand?

hrmm... that code is in the procedure for generating C calls for 64bit
intel systems
https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2541
is the  top of that routine

and width is defined right below the spot in question
https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764-L2774

it seems like the code/assertion likely predates michal's work? (eg, a
trick to make sure that genCCall64 doesn't get invoked by 32bit platforms?)

On Mon, Jul 23, 2018 at 8:54 PM Abhiroop Sarkar 
wrote:

> Hi Michal,
>
> In the tests that you have added to D4475, are all the tests running fine?
>
> On my machine, I was running the FFI tests(
> https://github.com/michalt/ghc/blob/int8/testsuite/tests/ffi/should_run/PrimFFIInt8.hs)
> and they seem to fail at a particular assert statement in the code
> generator.
>
> To be precise this one:
> https://github.com/michalt/ghc/blob/int8/compiler/nativeGen/X86/CodeGen.hs#L2764
>
> Upon commenting that assert the tests run fine. Am I missing something or
> is the failure expected?
>
> Thanks,
> Abhiroop
>
> On Mon, Jul 9, 2018 at 8:31 PM Michal Terepeta 
> wrote:
>
>> Just for the record, I've uploaded the changes to binary:
>> https://github.com/michalt/packages-binary/tree/int8
>>
>> - Michal
>>
>> On Wed, Jul 4, 2018 at 11:07 AM Michal Terepeta <
>> michal.terep...@gmail.com> wrote:
>>
>>> Yeah, if you look at the linked diff, there are a few tiny changes to
>>> binary that are necessary.
>>>
>>> I'll try to upload them to github later this week.
>>>
>>> Ben, is something blocking the review of the diff? I think I addressed
>>> all comments so far.
>>>
>>> - Michal
>>>
>>> On Wed, Jul 4, 2018 at 1:38 AM Abhiroop Sarkar 
>>> wrote:
>>>
 Hello Michal,

 I was looking at your diff https://phabricator.haskell.org/D4475  and
 there seems to be some changes that you perhaps made in the binary package 
 (
 https://phabricator.haskell.org/differential/changeset/?ref=199152=ignore-most).
 I could not find your version of binary on your github repos list. Is it
 possible for you to upload that so I can pull those changes?

 Thanks

 Abhiroop

 On Mon, May 28, 2018 at 10:45 PM Carter Schonwald <
 carter.schonw...@gmail.com> wrote:

> Abhiroop has the gist, though the size of word args for simd is more
> something we want to be consistent between 32/64 bit modes aka ghc
> targeting 32 or 64 bit intel with simd support :).  It would be best if 
> the
> unpack and pack operations that map to and from unboxed tuples where
> consistent and precise type wise.
>
>
>
> -Carter
>
> On May 28, 2018, at 5:02 PM, Abhiroop Sarkar 
> wrote:
>
> Hello Michal,
>
> My understanding of the issues are much lesser compared to Carter.
> However, I will state whatever I understood from discussions with him. Be
> warned my understanding might be wrong and Carter might be asking this for
> some completely different reason.
>
> > Out of curiosity, why do you need Word64#/Word32# story to be fixed
> for SIMD?
>
> One of the issues we are dealing with is multiple
> microarchitectures(SSE, AVX, AVX2 etc). As a result different
> microarchitectures has different ways of handling an 8 bit or 16 bit or 32
> bit unsigned integer. An example I can provide is the vector multiply
> instruction which has different semantics of multiplying and storing the
> first 16 bits of a 32 bit unsigned integer compared to the lower 16 bits
> for each of the elements in its SIMD registers. There will be some other
> intricacies around handling a byte sized integer or a 64 bit integer which
> will be different from the 32 bit version.
>
> Basically a 128 bit vector instruction will make some minor
> differences when dealing with 2 64 bit integers or 4 32 bit integer or 8 
> 16
> bit integers.
>
> So I think at the higher level we would want to be as precise as
> possible when specifying the datatypes and want things like Word8#,
> Word16#, Word32#, Word64# rather than a single ambiguous type like Word.
>
> One more example is this vector operation like : broadcastWord64X2# ::
>  Word#
> 
>  -> Word64X2#
> 
>  which
> should rather be broadcastWord64X2# :: Word64#
> 
>  -> Word64X2#
> 
>
> Another reason could be improving the space efficiency of packing
> various