Friday, 2023-01-20

*** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has quit IRC00:04
*** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has joined #libre-soc00:18
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC02:52
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc03:14
*** Ritish <Ritish!~Ritish@> has joined #libre-soc06:59
*** Ritish <Ritish!~Ritish@> has quit IRC07:53
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC08:16
ghostmansdprogrammerjake, lkcl, I came to conclusion that the original assembly is wrong.08:57
ghostmansdLet's assume I have instruction `sv.fdmadds 4,3,2,1`.08:58
ghostmansdIf I print `v30b_newfields`, I get `4,3,2,1`. Fields do not get reordered.08:59
ghostmansdThat is, fields which come as FRT,FRA,FRC,FRB are _not_ reordered into FRT,FRA,FRB,FRC, as prescribed by the specs.09:01
programmerjakelooks to me as though that code is wrong, i'd expect it to match powerisa fma ops, i'd guess whoever wrote it forgot that FRB and FRC are swapped in the assembly syntax09:01
programmerjakei'll leave it to luke to figure out what to do, i'm going to sleep, it's 1 am here.09:05
ghostmansdWell, the quick fix is to swap the v30b_newfields[2] and v30b_newfields[3]. But this will break some tests: test_sv_ffadds_dct and test_sv_fpmadds_fft.09:05
ghostmansdAh right, sorry, I always forget about the time zones.09:05
ghostmansdGood night!09:06
*** Ritish <Ritish!~Ritish@> has joined #libre-soc09:28
*** ghostmansd <ghostmansd!> has quit IRC09:33
*** lx0 <lx0!~lxo@gateway/tor-sasl/lxo> has joined #libre-soc09:39
*** lx0 <lx0!~lxo@gateway/tor-sasl/lxo> has quit IRC09:40
*** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has quit IRC09:40
*** octavius <octavius!> has joined #libre-soc09:41
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc09:53
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc09:53
*** lx0 <lx0!~lxo@gateway/tor-sasl/lxo> has joined #libre-soc09:56
lkcldo not whatever you do modify the ordering.10:46
lkclunder no circumstances.10:46
lkclthe DCT/FFT code is incredibly complex and the consequences of any modification will result in a massive cascade10:47
lkcli'm in no way prepared to go through that. it was many months in the first place.10:47
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC10:49
*** octavius <octavius!> has quit IRC11:00
*** Ritish <Ritish!~Ritish@> has quit IRC11:44
*** Ritish <Ritish!~Ritish@> has joined #libre-soc11:44
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc11:54
*** Ritish <Ritish!~Ritish@> has quit IRC12:25
*** octavius <octavius!> has joined #libre-soc12:38
octaviusI was looking at the hello_world.c (from Microwatt, also used in ls2), and the readl/writel functions used to load/store from/to memory using the Power instructions lwzcix and stwcix.12:38
octaviusFound these instructions in the spec (3.0C), section
octaviusThe spec says that these instructions produced defined results only if "the specified storage location is Caching Inhibited and Guarded."12:38
octaviusDoes this mean that the storage locations must not be connected to the CPU via a cache?12:38
octaviusThe programming note does say these ld/st instructions are "used to permit a control register on an I/O device to be accessed without permitting the corresponding storage location to be copied into the caches"12:38
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC13:27
*** ghostmansd <ghostmansd!> has joined #libre-soc14:00
lkclno, it means that *if* there is a cache it must be write-thru14:19
octaviusAh ok14:23
*** octavius <octavius!> has quit IRC14:28
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc14:32
*** octavius <octavius!> has joined #libre-soc14:36
*** octavius <octavius!> has quit IRC14:38
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC14:39
ghostmansdlkcl, please stop these "in no way"14:44
ghostmansdAgain: this contradicts to spec.14:44
ghostmansdPlease fix the test accordingly, it is a show stopper.14:45
ghostmansdUnless the operand order is FRT,FRA,FRB,FRC, your assembler code is wrong, and so are the tests.14:46
ghostmansdYou put FRB where FRC resides. Please consider investigating the topic before submitting the reply.14:47
ghostmansdThe topic starts here:
ghostmansdIf you have other options, like reordering the operands, let me know.14:54
ghostmansdOtherwise this is counterproductive and brings nothing useful to dicussion.14:54
lkcli am not going to change the instruction nor the unit test because the cascade of dependencies is so vast and complex15:35
lkclthe unit test passes, therefore there has to be an absolute show-stopper reason15:36
lkclthere is a *convention* from IBM about the order of operands and what they mean15:36
lkclif i made a mistake by not following that convention, then it's too late now. i'm not prepared to do the massive amount of work needed to change it15:38
lkclthe reason is that these instructions are closely inter-related and also tied in to the REMAP system15:38
markoswon't IBM themselves complain about the spec not being followed?15:43
ghostmansdWell, they likely won't, these instructions are ours15:53
ghostmansdAnd that's the exact reason why I asked whether we can change the operands order for the first place15:54
ghostmansdBut it appears that "no ways" start before even reading the discussion.15:54
ghostmansdI'm adding a big FIXME. And leaving it with you.15:55
markoswell, I'm of the opinion that such changes need to happen early if at all15:55
ghostmansdI don't have either time or patience to deal with it now.15:55
markosI wouldn't mind doing the fix myself on dct/fft, if anything it would help me get me understand this part of the code, which I want to anyway, if I am to contribute anything back to it15:56
markosargh, s/help me get me/help me/15:57
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc15:59
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC16:06
*** Ritish <Ritish!~Ritish@> has joined #libre-soc16:07
ghostmansdmarkos, the latest commit into insndb branch shows what happens:;a=blobdiff;f=src/openpower/decoder/;h=1939acbfaed14afa1025e17ca88f182601f312f0;hp=a53e2195b7874bacb60475c93322f84ff6f01e5a;hb=b2dff8d50464e374577a8279298d90d378960450;hpb=8b434d31042fa489b4a19dade4bbc5a170265dfa16:12
ghostmansdThe logic you see here follows the original assembler from master branch:;a=blob;f=src/openpower/sv/trans/;h=c61e92833d696d46c50bf715c898b33a3100aa49;hb=refs/heads/master#l154116:13
ghostmansdNow, starting from this line:;a=blob;f=src/openpower/decoder/;h=1939acbfaed14afa1025e17ca88f182601f312f0;hb=1939acbfaed14afa1025e17ca88f182601f312f0#l62016:14
ghostmansdWe add several custom instructions which have some operands with special logic which needs to be explicitly enabled.16:14
ghostmansdffmadds et al. use FMAOperandFRB class for FRB operand and FMAOperandFRC for FRC operands16:15
ghostmansdThese all come from openpower/isa/svfparith.mdwn tables; when we parse this file, we obtain a list of operands (including "static" ones, like Rc)16:16
ghostmansdEach of these operands then is wrapped into some class implementing assembly/disassembly logic.16:16
ghostmansdAnd some operands are custom. They can be custom for a specific instruction (custom_insns array), some can be always specific (custom_fields), and some are always specific and marked as immediates (that is, they are used together in X(Y) syntax).16:26
ghostmansdBut, before changing this part, please discuss it with Luke.16:26
markosnot going to change anything yet, anyway, would have to study the code extensively to understand how it works, before that happens16:30
*** Ritish <Ritish!~Ritish@> has quit IRC16:33
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc16:42
ghostmansd[m]markos, sure! Ping me if you need some help or have some issues with the assembly part.17:04
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC17:15
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc17:43
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC17:55
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc17:57
*** toshywoshy <toshywoshy!> has quit IRC17:59
*** toshywoshy <toshywoshy!> has joined #libre-soc18:00
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC18:11
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc18:30
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC18:45
lkclmarkos, well in the case of the dual-butterfly ones, i think a case can be made that it doesn't matter. or, because implicit-FRS/RS is involved, those conventions can be discarded20:17
lkclthere is actually a reason:20:17
lkclby arranging the registers in the order in which they are arranged20:17
lkclone single svshape instruction - specifying which of the 4 shapes apply to which of 5 registers - can cover *more than one instruction*20:18
lkclif the registers are rearranged "to suit conventions" it requires *TWO* svshape instructions20:18
markosas usual, if there is a reasoning to break convention then it's fine, conventions are made to be broken anyway :)20:19
lkclgiven that that represents a whopping 33% increase in instructions in a couple of cases, that's "a good case for disregarding whatever historic conventions were used formerly"20:19
markosmy volunteering was just in the case that it was made by accident and you don't want to mess with the particular part of the code, it's not that I *want* to fix it20:20
lkclghostmansd, it's that i have to recall a massive stack of heavily in-depth coding which because of my memory problems it takes far longer to recall what was even done than you or anyone else would expect20:21
markoseven if IBM complained for breaking convension, avoiding 33% increase in instruction count is a pretty good argument to be made20:21
lkclthat was a 3-instruction case (maybe 4), where the extra svshape instruction obviously increases to 4 (maybe 5)20:22
lkclthere's a couple like that in the DCT/FFT code, where by a really nice coincidence as i was designing ffmadds (etc.) a result turned out to be in exactly the right-named register as an input operand needing exactly the same svshape...20:23
lkclthe required svshape could be assigned to FRB *once and only once*20:23
lkcl(or whatever)20:23
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc20:23
markosperhaps it would be best to add this note there to avoid revisiting the issue in the future?20:24
lkclsome similar logic went into the design of the LD/ST and shift-and-mask instructions as far back as the early 90s when Power ISA was first designed20:24
lkclthey had absolutely no way they could duplicate a shift-and-mask unit inside LD/ST - far too many gates20:25
lkclso they arranged RA RB RC RS and RT to be "broadcast lanes"20:25
lkclthen *micro-coded* LD/ST as a *pair* of operations20:25
lkclfirst operation: get the data20:25
lkclsecond operation: broadcast the *unaligned* data onto one of the "operand lane" buses as a *direct* input to the shift-and-mask pipeline, BYPASSING the register file but using the exact same input20:26
lkclthis is why you have the weird register names in the first place20:27
lkcloperand names20:27
lkclghostmansd, regarding those mappings: i cannot talk about what is being discussed confidentially in the OPF ISA WG. that's as much as i can say until a new revision is released20:31
lkclwe _can_ talk again about the mappings when the new revision is released.20:32
ghostmansd[m]At this point, I want to ensure that things work (they already mostly do, but some tests still need to be fixed). If I find some issue in the underlying code, I will for now resort to following the same behavioral patterns as the old code did.21:56
ghostmansd[m]But still I cannot avoid raising these topics, because sometimes these topics are real issues.21:56
ghostmansd[m]However, one point. Before answering to any thread, please _read_ it. I literally spent three months of debugging and developing this, and I cannot treat the reply "in no way" or "I don't have time to explain yet" as a satisfactory one.21:59
ghostmansd[m]This is unethical considering all the time I spent and all the efforts and patience I had.22:01
ghostmansd[m]Even restraining from reply would have been better.22:01
ghostmansd[m]That said, no hard feelings, just please keep this in mind. Thank you.22:02
*** ghostmansd[m] <ghostmansd[m]!> has quit IRC22:57
*** ghostmansd[m] <ghostmansd[m]!> has joined #libre-soc22:58

Generated by 2.17.1 by Marius Gedminas - find it at!