| ghostmansd[m] | Sorry, I should've written this not in form of bitwise operations, but rather via indexing | 08:40 |
|---|---|---|
| ghostmansd[m] | I meant, `sign = value[XLEN - 1]` is not OK in PPC, right? | 08:41 |
| ghostmansd[m] | Also, regarding repo push: I mean libreriscv repo | 08:41 |
| programmerjake | depends on if your using bits numbered lsb0 (like nmigen and most software) or msb0 (openpower isa spec) | 08:52 |
| ghostmansd[m] | lkcl: Are XLCASTS/XLCASTU in XLEN branch OK? | 08:52 |
| ghostmansd[m] | programmerjake: yes, that's what I mean, thank you for putting it clear :-) | 08:53 |
| programmerjake | if it's numbered lsb0, then that is the sign bit, if it's msb0, then that bit is actually equivalent to `value & 1` | 08:53 |
| programmerjake | if your writing openpower isa spec pseudo-code, it uses msb0 | 08:54 |
| programmerjake | afaict, the functions should be like: | 09:02 |
| programmerjake | def XLCASTU(self, value): | 09:02 |
| programmerjake | def XLCASTU(self, value): | 09:12 |
| programmerjake | bits = min(self.XLEN, value.bits) | 09:12 |
| programmerjake | return SelectableInt(value.value & ((1 << bits) - 1), self.XLEN) | 09:12 |
| ghostmansd | programmerjake: and XLCASTS should be the same but with sign extension? | 09:17 |
| lkcl | <ghostmansd[m]> I meant, `sign = value[XLEN - 1]` is not OK in PPC, right? | 09:18 |
| ghostmansd | def XLCASTS(x): | 09:18 |
| ghostmansd | bits = min(value.bits, self.XLEN) | 09:18 |
| ghostmansd | value = exts(value.value, bits) | 09:18 |
| ghostmansd | return SelectableInt(value & ((1 << bits) - 1), self.XLEN) | 09:18 |
| lkcl | that's incorrect | 09:18 |
| lkcl | `sign = value[XLEN - 1]` ***is*** OK in PPC | 09:18 |
| lkcl | it cannot be any other way | 09:18 |
| lkcl | to be otherwise would be catastrophic and disastrous for any programs and compilers | 09:18 |
| lkcl | programs would be utterly non-portable to the Power ISA | 09:19 |
| lkcl | therefore, logically, we may deduce that it *has* to be exactly as for all and any other architecture that has 2's-complement integer arithmetic | 09:20 |
| lkcl | ghostmansd, haven't looked yet | 09:20 |
| programmerjake | well, lkcl, you're forgetting that on ppc bits are numbered from 0 at the *msb* end to xlen -1 at the lsb end. the sign is still always at the *msb* end aka. bit 0 on ppc | 09:20 |
| lkcl | ahh yeah, that's a Power ISA spec issue | 09:21 |
| lkcl | not a hardware issue | 09:21 |
| ghostmansd[m] | Yes, I'm about specs | 09:21 |
| lkcl | the MSB is always where the sign-extension must occur | 09:21 |
| lkcl | ghostmansd[m], ok. | 09:21 |
| lkcl | so yes, in IBM's totally-backwards LSB0-numbering, you want the bit numbered zero, aka the MSB, as the sign bit | 09:22 |
| ghostmansd[m] | Pushed fixedlogical changes and updated XLCASTS/XLCASTU | 09:22 |
| lkcl | it looks like the only "damage" to the wiki git repository is a claim that this material is copyrighted (written) by you | 09:23 |
| lkcl | https://git.libre-soc.org/?p=libreriscv.git;a=commitdiff;h=a94a093c9ea01ea19d09ebea82f5330fc28d994b | 09:23 |
| lkcl | oh wait | 09:24 |
| lkcl | it still says "author me" | 09:24 |
| lkcl | but submitter you | 09:24 |
| ghostmansd[m] | Yes :-) | 09:24 |
| lkcl | that's ok | 09:24 |
| programmerjake | def XLCASTS(self, value): | 09:24 |
| programmerjake | return SelectableInt(exts(value.value, self.XLEN), self.XLEN) | 09:24 |
| ghostmansd[m] | But the commit tells that you've updated RFPs :-D | 09:24 |
| ghostmansd[m] | So I wondered if it's OK to do force-push to drop it | 09:25 |
| programmerjake | exts already does the hard work for us | 09:25 |
| lkcl | argh argh there's a rebase error on the server repo | 09:26 |
| ghostmansd[m] | programmerjake: thank you, pushed | 09:26 |
| lkcl | ok sorted | 09:29 |
| programmerjake | the matrix bridge was borked for the last few minutes... | 09:30 |
| lkcl | ghostmansd[m]: https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/helpers.py;h=d51305b96ad396cbad760e8d69d14ac6393206a2;hb=b9a854e56c5c80225508d43069ae666b6376c94b#l36 | 09:30 |
| lkcl | the exts function does everything arithmetically, so you don't even need to know the numbering (LSB0 ordering) | 09:31 |
| lkcl | the only reason you ever need to know the LSB0 ordering is when trying to *directly* access bits | 09:32 |
| lkcl | x = SelectableInt() | 09:32 |
| lkcl | x = SelectableInt(8) | 09:32 |
| lkcl | x[0] --> the *SEVENTH* bit of x | 09:32 |
| lkcl | if you use SelectableInt in an *arithmetic* way, you *DO NOT* need to know that | 09:32 |
| lkcl | at all | 09:32 |
| programmerjake | well, how the XLCASTS function I posted uses exts, you can't even access bits using indexing, since it only gets a python int | 09:33 |
| programmerjake | in exts | 09:33 |
| lkcl | SelectableInt behaves precisely and exactly like a standard 2's-complement arithmetic object | 09:33 |
| ghostmansd | extsb | 09:34 |
| ghostmansd | RA <- XLCASTS(RS & 0xFF) | 09:34 |
| ghostmansd | extsh | 09:35 |
| ghostmansd | RA <- XLCASTS(RS & 0xFFFF) | 09:35 |
| lkcl | if both the value and the width received by exts are python ints, then it will actually behave exactly as expected. | 09:35 |
| programmerjake | sort-of...the constructor makes it act like a unsigned modular integer rather than a 2's complement modular integer | 09:35 |
| ghostmansd | extsb can also be written as `RA <- XLCASTS(RS[XLEN-8:XLEN-1])` | 09:35 |
| lkcl | due to the Liskov Substitution Principle it will work for both | 09:35 |
| ghostmansd | I'm not sure how to write extsh in the same fashion | 09:35 |
| lkcl | ahh yeah i forgot about that | 09:35 |
| lkcl | good catch, jacob | 09:36 |
| * lkcl thinking | 09:37 | |
| ghostmansd | checking ./src/soc/fu/logical/test/test_pipe_caller.py | 09:38 |
| ghostmansd | ok, forgot self... | 09:38 |
| programmerjake | extsb: RT <- XLCASTS(EXTS64((RA))[56:63]) | 09:40 |
| programmerjake | extsh: RT <- XLCASTS(EXTS64((RA))[48:63]) | 09:40 |
| lkcl | what i suggest is just use EXTS((RA), 8) | 09:40 |
| lkcl | what i suggest is just use EXTS((RA), 16) | 09:40 |
| lkcl | what i suggest is just use EXTS((RA), 32) | 09:40 |
| lkcl | for extsb, extsh and extsw respectively | 09:40 |
| lkcl | and we sort it out behind the scenes | 09:40 |
| lkcl | get EXTS() to perform the [optionally] required truncation | 09:41 |
| lkcl | it doesn't need spec changes | 09:41 |
| lkcl | and the implementation details are hidden and do not need to be declared to OPF ISA WG | 09:42 |
| programmerjake | lkcl: that could totally work, except that the result of EXTS would be 8/16/32 bits for extsb/h/w rather than XLEN | 09:42 |
| lkcl | other than a terse note, "EXTS must perform truncation if XLEN=8" | 09:42 |
| lkcl | EXTS is implemented as setting the width to 256 to indicate "unlimited length" | 09:43 |
| lkcl | which is then detected by subsequent operations and *replaced* with the length at which it is used | 09:43 |
| programmerjake | ah, ok, sounds good! | 09:44 |
| lkcl | long story behind that one | 09:44 |
| lkcl | consequently, when copying the result into the GPR, which will have a length set on it of 8, the truncation will automatically occur at that point | 09:44 |
| programmerjake | wait, can extsh have different src and dest xlen? elwidsrc and elwiddest? | 09:45 |
| programmerjake | if so, we need to do sign extension when converting from xlensrc to 8/16/32, and sign extension again when converting to xlendest | 09:46 |
| programmerjake | icr if we have two xlen | 09:46 |
| lkcl | i'm hoping that the decision to "operate at maximum XLEN" | 09:47 |
| lkcl | will result in the source/dest XLEN *not* having to be exposed to the pseudocode | 09:48 |
| lkcl | but sorted out as a "pre-analysis phase" followed by "execute at one and only one XLEN" followed by "post-analysis and potential truncation phase" | 09:48 |
| programmerjake | lkcl? | 09:49 |
| lkcl | if that algorithm's insufficient then sigh we have to bite the bullet and add an SLEN (source XLEN) and DLEN (dest XLEN) | 09:49 |
| lkcl | which i reaaaallly would, obviously, greatly prefer not to do | 09:49 |
| lkcl | trying to explain it to the OPF ISA WG would be... fraught | 09:49 |
| * lkcl afk, got up too early | 09:50 | |
| programmerjake | k, though not everything will match user's expected/assumed semantics then | 09:50 |
| programmerjake | too sleepy to come up with any examples at the moment tho | 09:50 |
| programmerjake | btw, today's my mom's actual bday, sunday was just the bday party | 09:52 |
| programmerjake | well, i'm going to sleep, ttyl | 09:54 |
| programmerjake | good luck with the xlen stuff ghostmansd! | 09:54 |
| ghostmansd[m] | Thanks! | 10:13 |
| ghostmansd[m] | lkcl: if you _really_ want to go with inheritance from ISACallerHelper, then you _have_ to call super().__init__() if you have custom __init__ in inherited class. | 12:31 |
| ghostmansd | That's one of things we missed. The next one is that XLEN should also be transformed to self.XLEN, it seems. | 12:35 |
| ghostmansd | I frankly don't even know how it works now. | 12:35 |
| ghostmansd | Ah, OK, likely via this big-neat self.namespace. OK, but it'd better if XLEN is prefixed with `self.` upon obtaining it. | 12:46 |
| ghostmansd | Because, well, from helpers it _is_ self.XLEN. | 12:47 |
| lkcl | ghostmansd, ah whoops | 13:16 |
| lkcl | yes, self.XLEN is the next thing | 13:17 |
| lkcl | the idea is that before each and every instruction is executed, self.XLEN must be set. | 13:17 |
| ghostmansd | I've already updated the parser | 13:18 |
| ghostmansd | And added super() | 13:18 |
| ghostmansd | And some other stuff | 13:18 |
| ghostmansd | in xlen branch | 13:18 |
| ghostmansd | 11 failures in test_issuer.py, including svp64 | 13:18 |
| ghostmansd | Mostly something related to selectable int | 13:18 |
| ghostmansd | I guess it has to do with self.XLEN, but haven't figured why it breaks yet. | 13:20 |
| lkcl | ok let me have a go | 13:25 |
| lkcl | can you attach an example stack trace or list precisely one and only one instruction that fails | 13:25 |
| lkcl | (so i don't have to run them all for 25 minutes) | 13:26 |
| ghostmansd | File "/home/ghostmansd/src/openpower-isa/src/openpower/decoder/isa/fixedarith.py", line 620, in op_divweu | 13:29 |
| ghostmansd | if eq(divisor, [0] * self.XLEN): | 13:29 |
| ghostmansd | File "/home/ghostmansd/src/openpower-isa/src/openpower/decoder/helpers.py", line 143, in eq | 13:29 |
| ghostmansd | return onebit(a == b) | 13:29 |
| ghostmansd | File "/home/ghostmansd/src/openpower-isa/src/openpower/decoder/selectable_int.py", line 413, in __eq__ | 13:29 |
| ghostmansd | assert False | 13:29 |
| ghostmansd | AssertionError | 13:29 |
| ghostmansd | I guess that relates to the fact that we have 64 zeroes instead of 1 int :-) | 13:29 |
| ghostmansd | I think I've found the issue | 13:29 |
| ghostmansd | Luke, do you do some magic after XLEN is obtained from self.namespace? | 13:30 |
| ghostmansd | You do convert it to selectable int, don't you? | 13:30 |
| ghostmansd | self.XLEN is a simple int, I guess this is where the issue comes from. Let me check. | 13:30 |
| lkcl | yes. | 13:32 |
| lkcl | that sounds likely | 13:33 |
| lkcl | ah no | 13:33 |
| lkcl | wrong file, doh | 13:33 |
| lkcl | no, no magic | 13:33 |
| lkcl | XLEN is set to 64 as an int in the self.namespace dict | 13:34 |
| ghostmansd | Yes, but in the places you access self.namespace, don't you do magic? | 13:34 |
| lkcl | @inject | 13:34 |
| ghostmansd | Because [0]*64 is 64 zeroes | 13:35 |
| lkcl | it doesn't actually involve *access* to self.namespace | 13:35 |
| lkcl | the dictionary *items* of self.namespace are merged into the globals() used by the function | 13:35 |
| ghostmansd | And, I suspect, [0]*SelectableInt(64) is 0b.....0 (i.e. 64 bits) | 13:35 |
| ghostmansd | OK, let me check whether conversion helps | 13:35 |
| lkcl | which is a shockingly-bad hack but works | 13:35 |
| lkcl | ermmm.. ermermerm... | 13:36 |
| lkcl | hold on | 13:36 |
| lkcl | waitwaitwait | 13:36 |
| lkcl | [0]*self.XLEN should never have made it into the compiler output | 13:36 |
| lkcl | it should have been identified *by the compiler* and *replaced* with | 13:36 |
| ghostmansd | OK, that's where we lost :-) | 13:37 |
| lkcl | selectconcat(0, self.XLEN) | 13:37 |
| ghostmansd | I guess selectconcat only works with Name?... | 13:37 |
| ghostmansd | Because in one of commits it became Attribute | 13:37 |
| lkcl | nono, it's not selectconcat that's the problem | 13:37 |
| ghostmansd | that is, when we see XLEN, we convert it into self.XLEN | 13:37 |
| ghostmansd | and do it via Name + Attribute combo | 13:38 |
| lkcl | it's the compiler that outputs that function as a simple text string | 13:38 |
| lkcl | that sounds about right | 13:38 |
| ghostmansd | yes yes, I'm talking about compiler | 13:38 |
| lkcl | identify_sint_mul_pattern | 13:38 |
| ghostmansd | Aha | 13:43 |
| ghostmansd | XLEN is no longer ast.Name | 13:43 |
| ghostmansd | But an ast.Attribute | 13:43 |
| lkcl | or, it's an ast.Attribute with an id "self" | 13:48 |
| lkcl | and a value which is an ast.Name, where that ast.Name's id is "XLEN" | 13:48 |
| lkcl | btw you notice i drop a whole stack of tests into power_pseudo.py, right? | 13:49 |
| lkcl | feel free to add one "concat_test4 = [0]*self.XLEN" | 13:49 |
| lkcl | actually i'll do that now... | 13:49 |
| lkcl | actually, don't need to | 13:50 |
| lkcl | it's concat_test3 | 13:50 |
| lkcl | ghostmansd, git pull. https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=be99ee688c680e0fc7b9ce81f53bcbe582500429 | 13:51 |
| lkcl | you can back out setting XLEN to SelectableInt | 13:51 |
| ghostmansd | ok will continue later on this | 14:16 |
| lkcl | ack | 14:41 |
| lkcl | got PartitionedSignal to test | 14:41 |
| ghostmansd | ok, making the thing an attribute does work! | 20:33 |
| ghostmansd | exactly as I hoped :-) | 20:33 |
| ghostmansd | test_issuer.py works on my side | 20:33 |
| ghostmansd | (w/o arguments, so svp64 is included) | 20:34 |
| ghostmansd | let me know if I need more tests to check | 20:34 |
| ghostmansd | I suggest moving these into the master | 20:34 |
| ghostmansd | lkcl, ^ | 20:34 |
| ghostmansd | lkcl, I merged these, since they didn't break any of test_issuer.py tests | 21:14 |
| mikolajw | hi, checking if Matrix bridge works, can you read what I'm writing? | 21:34 |
| mikolajw | ok, I see from the logs it works | 21:35 |
| ghostmansd[m] | Yes, it works :-) | 21:56 |
| ghostmansd[m] | I can see your messages, mikolajw | 21:56 |
| mikolajw | thanks | 22:00 |
| programmerjake | :) | 22:17 |
| lkcl | ghostmansd[m], excellent, will do tomorrow. long day today | 22:41 |
| lkcl | oo that's a lot :) | 22:41 |
| lkcl | ghostmansd[m], look at the constructor for SelectableInt. | 22:44 |
| lkcl | it already performs "mask = (1<<bits)-1, self.value = value & mask" | 22:44 |
| lkcl | so XCASTU has no need to repeat that | 22:44 |
| ghostmansd[m] | lkcl: let's be explicit :-) | 22:45 |
| ghostmansd[m] | After all, specs have no idea of selectable int | 22:45 |
| lkcl | a comment would suffice | 22:45 |
| lkcl | the spec needs to be explicit | 22:45 |
| ghostmansd[m] | Ah, you mean the impl | 22:45 |
| lkcl | the implementation is up to us. | 22:45 |
| ghostmansd[m] | Not the call itself | 22:45 |
| lkcl | yes | 22:45 |
| ghostmansd[m] | Ok, will check tomorrow | 22:45 |
| lkcl | a comment, def XLCASTU(self, value): | 22:45 |
| ghostmansd[m] | Too tired today | 22:45 |
| lkcl | # SelectableInt already masks out the bits | 22:46 |
| lkcl | i hear ya | 22:46 |
| lkcl | me too. done for the day | 22:46 |
| ghostmansd[m] | I liked that task | 22:46 |
| lkcl | :) | 22:46 |
| ghostmansd[m] | Initially it seemed to be quite straightforward | 22:46 |
| ghostmansd[m] | But we did a lot in scope of it | 22:47 |
| ghostmansd[m] | Basically we affected the whole generation process | 22:47 |
| ghostmansd[m] | I'll simplify XLCASTU and proceed into extsb/extsh | 22:48 |
| ghostmansd[m] | What do we have left after these? | 22:48 |
| ghostmansd[m] | Also, any news from NLnet? | 22:49 |
| ghostmansd[m] | They gone silent :-) | 22:49 |
| ghostmansd[m] | IDK if you saw it in mailing list, but 3mdeb also got affected | 22:49 |
| ghostmansd[m] | Also I had to re-submit one of RFPs, hope it won't introduce more confusion | 22:51 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!