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/P... ) 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-L1... 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.schonwald@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#L... 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#L...

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/Prim...) 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#L...

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.terepeta@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.terepeta@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&whitespace=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.schonwald@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# >> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >> -> Word64X2# >> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... which >> should rather be broadcastWord64X2# :: Word64# >> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >> -> Word64X2# >> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >> >> Another reason could be improving the space efficiency of packing >> various datatypes. This was explained in some detail in this comment: >> https://github.com/ghc-proposals/ghc-proposals/pull/74#issuecomment-33395155... >> >> Thanks, >> Abhiroop >> >> On Mon, May 28, 2018 at 6:06 PM, Michal Terepeta < >> michal.terepeta@gmail.com> wrote: >> >>> Hey Carter, >>> >>> Yeah, I'm totally snowed under with personal stuff at the moment, >>> so not much time to hack on >>> anything. I've managed to make some progress on >>> https://phabricator.haskell.org/D4475 (prototype >>> that adds Int8#/Word8#; waiting for another round of review). >>> Sadly June still looks a bit crazy for >>> me, so I can't promise anything for Word64#/Word32# (and >>> Int64#/Int32#). I hope to have some more >>> time in July. >>> >>> If this is blocking you, then please don't wait for me and go >>> ahead (my diff for Int8#/Word8# might >>> be a decent starting point to see which places might need changes). >>> >>> Out of curiosity, why do you need Word64#/Word32# story to be >>> fixed for SIMD? Cmm already has >>> separate types for vector types (see TyCon.hs and VecRep >>> constructor of PrimRep). >>> >>> Cheers! >>> >>> - Michal >>> >>> PS. Are you coming to ZuriHac by any chance? >>> >>> On Mon, May 28, 2018 at 12:37 AM Carter Schonwald < >>> carter.schonwald@gmail.com> wrote: >>> >>>> Hey Michal! >>>> So i'm mentoring Abhiroop around getting SIMD all nice and >>>> awesome, and I realized we still dont quite yet have the nice Word32# and >>>> Word64# etc story in ghc as yet. >>>> >>>> Whats the status on that and or is there a way we can help get >>>> that over the hump for ghc? These different sized in register ints and >>>> friends have a tight partnership with any nice SIMD iterating over the >>>> summer, and hopefully helps inform the design >>>> >>>> -Carter >>>> >>> >> >> >> -- >> Kloona - Coming Soon! >> >> > > -- > Kloona - Coming Soon! >

-- Kloona - Coming Soon!

-- Kloona - Coming Soon!

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/P... )

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-L1... 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.terepeta@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.schonwald@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#L... 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#L...

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/Prim...) 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#L...

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.terepeta@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.terepeta@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 < >> asiamgenius@gmail.com> 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&whitespace=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.schonwald@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 < >>>> asiamgenius@gmail.com> 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# >>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>> -> Word64X2# >>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... which >>>> should rather be broadcastWord64X2# :: Word64# >>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>> -> Word64X2# >>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>> >>>> Another reason could be improving the space efficiency of packing >>>> various datatypes. This was explained in some detail in this comment: >>>> https://github.com/ghc-proposals/ghc-proposals/pull/74#issuecomment-33395155... >>>> >>>> Thanks, >>>> Abhiroop >>>> >>>> On Mon, May 28, 2018 at 6:06 PM, Michal Terepeta < >>>> michal.terepeta@gmail.com> wrote: >>>> >>>>> Hey Carter, >>>>> >>>>> Yeah, I'm totally snowed under with personal stuff at the >>>>> moment, so not much time to hack on >>>>> anything. I've managed to make some progress on >>>>> https://phabricator.haskell.org/D4475 (prototype >>>>> that adds Int8#/Word8#; waiting for another round of review). >>>>> Sadly June still looks a bit crazy for >>>>> me, so I can't promise anything for Word64#/Word32# (and >>>>> Int64#/Int32#). I hope to have some more >>>>> time in July. >>>>> >>>>> If this is blocking you, then please don't wait for me and go >>>>> ahead (my diff for Int8#/Word8# might >>>>> be a decent starting point to see which places might need >>>>> changes). >>>>> >>>>> Out of curiosity, why do you need Word64#/Word32# story to be >>>>> fixed for SIMD? Cmm already has >>>>> separate types for vector types (see TyCon.hs and VecRep >>>>> constructor of PrimRep). >>>>> >>>>> Cheers! >>>>> >>>>> - Michal >>>>> >>>>> PS. Are you coming to ZuriHac by any chance? >>>>> >>>>> On Mon, May 28, 2018 at 12:37 AM Carter Schonwald < >>>>> carter.schonwald@gmail.com> wrote: >>>>> >>>>>> Hey Michal! >>>>>> So i'm mentoring Abhiroop around getting SIMD all nice and >>>>>> awesome, and I realized we still dont quite yet have the nice Word32# and >>>>>> Word64# etc story in ghc as yet. >>>>>> >>>>>> Whats the status on that and or is there a way we can help get >>>>>> that over the hump for ghc? These different sized in register ints and >>>>>> friends have a tight partnership with any nice SIMD iterating over the >>>>>> summer, and hopefully helps inform the design >>>>>> >>>>>> -Carter >>>>>> >>>>> >>>> >>>> >>>> -- >>>> Kloona - Coming Soon! >>>> >>>> >>> >>> -- >>> Kloona - Coming Soon! >>> >>

-- Kloona - Coming Soon!

-- Kloona - Coming Soon!

-- Kloona - Coming Soon!

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.schonwald@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/P... )

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-L1... 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 < michal.terepeta@gmail.com> 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.terepeta@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.schonwald@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#L... > 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#L... > > 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 < > asiamgenius@gmail.com> 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/Prim...) >> 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#L... >> >> 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.terepeta@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.terepeta@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 < >>>> asiamgenius@gmail.com> 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&whitespace=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.schonwald@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 < >>>>>> asiamgenius@gmail.com> 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# >>>>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>>>> -> Word64X2# >>>>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... which >>>>>> should rather be broadcastWord64X2# :: Word64# >>>>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>>>> -> Word64X2# >>>>>> https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Wo... >>>>>> >>>>>> Another reason could be improving the space efficiency of >>>>>> packing various datatypes. This was explained in some detail in this >>>>>> comment: >>>>>> https://github.com/ghc-proposals/ghc-proposals/pull/74#issuecomment-33395155... >>>>>> >>>>>> Thanks, >>>>>> Abhiroop >>>>>> >>>>>> On Mon, May 28, 2018 at 6:06 PM, Michal Terepeta < >>>>>> michal.terepeta@gmail.com> wrote: >>>>>> >>>>>>> Hey Carter, >>>>>>> >>>>>>> Yeah, I'm totally snowed under with personal stuff at the >>>>>>> moment, so not much time to hack on >>>>>>> anything. I've managed to make some progress on >>>>>>> https://phabricator.haskell.org/D4475 (prototype >>>>>>> that adds Int8#/Word8#; waiting for another round of review). >>>>>>> Sadly June still looks a bit crazy for >>>>>>> me, so I can't promise anything for Word64#/Word32# (and >>>>>>> Int64#/Int32#). I hope to have some more >>>>>>> time in July. >>>>>>> >>>>>>> If this is blocking you, then please don't wait for me and go >>>>>>> ahead (my diff for Int8#/Word8# might >>>>>>> be a decent starting point to see which places might need >>>>>>> changes). >>>>>>> >>>>>>> Out of curiosity, why do you need Word64#/Word32# story to be >>>>>>> fixed for SIMD? Cmm already has >>>>>>> separate types for vector types (see TyCon.hs and VecRep >>>>>>> constructor of PrimRep). >>>>>>> >>>>>>> Cheers! >>>>>>> >>>>>>> - Michal >>>>>>> >>>>>>> PS. Are you coming to ZuriHac by any chance? >>>>>>> >>>>>>> On Mon, May 28, 2018 at 12:37 AM Carter Schonwald < >>>>>>> carter.schonwald@gmail.com> wrote: >>>>>>> >>>>>>>> Hey Michal! >>>>>>>> So i'm mentoring Abhiroop around getting SIMD all nice and >>>>>>>> awesome, and I realized we still dont quite yet have the nice Word32# and >>>>>>>> Word64# etc story in ghc as yet. >>>>>>>> >>>>>>>> Whats the status on that and or is there a way we can help >>>>>>>> get that over the hump for ghc? These different sized in register ints and >>>>>>>> friends have a tight partnership with any nice SIMD iterating over the >>>>>>>> summer, and hopefully helps inform the design >>>>>>>> >>>>>>>> -Carter >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Kloona - Coming Soon! >>>>>> >>>>>> >>>>> >>>>> -- >>>>> Kloona - Coming Soon! >>>>> >>>> >> >> -- >> Kloona - Coming Soon! >> >

-- Kloona - Coming Soon!

-- Kloona - Coming Soon!

-- Kloona - Coming Soon!