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 <[email protected]> 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 < > [email protected]> 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 <[email protected]> >> 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 <[email protected]> >>> 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 < >>>> [email protected]> 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 < >>>>> [email protected]> 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 < >>>>>> [email protected]> 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 < >>>>>>> [email protected]> 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 < >>>>>>>> [email protected]> 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 < >>>>>>>>> [email protected]> 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 < >>>>>>>>>> [email protected]> 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 < >>>>>>>>>>> [email protected]> 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 < >>>>>>>>>>>> [email protected]> 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:Word-35-> >>>>>>>>>>>> -> Word64X2# >>>>>>>>>>>> <https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Word64X2-35-> >>>>>>>>>>>> which >>>>>>>>>>>> should rather be broadcastWord64X2# :: Word64# >>>>>>>>>>>> <https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Word-35-> >>>>>>>>>>>> -> Word64X2# >>>>>>>>>>>> <https://hackage.haskell.org/package/ghc-prim-0.5.2.0/docs/GHC-Prim.html#t:Word64X2-35-> >>>>>>>>>>>> >>>>>>>>>>>> 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-333951559 >>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> Abhiroop >>>>>>>>>>>> >>>>>>>>>>>> On Mon, May 28, 2018 at 6:06 PM, Michal Terepeta < >>>>>>>>>>>> [email protected]> 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 < >>>>>>>>>>>>> [email protected]> 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! >
_______________________________________________ ghc-devs mailing list [email protected] http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
