| stekern | so SI = signed32 and DI = signed64, which explains why that change breaks on 32-bit machines, since the incoming value is a long | 05:27 |
|---|---|---|
| juliusb | ?? | 12:42 |
| juliusb | I reckon it's worth a post to the CGEN mailing list about it | 12:43 |
| juliusb | It's not too active, but you may get a bite | 12:43 |
| stekern | juliusb: I'm not sure how I should phrase a question there | 12:47 |
| stekern | did you see the post to our ML? | 12:47 |
| juliusb | yep | 12:56 |
| juliusb | sorry, I didn't fully get it, the problem was caused by the SI being changed to DI in the opcodes/or1k-ibld.c file, right? | 13:01 |
| stekern | yup | 13:02 |
| juliusb | and your fix was to change cpu/or1korbis.cpu in the way you pasted, where IAI should be SI | 13:02 |
| stekern | yup | 13:02 |
| stekern | but the commit we looked at yesterday (the one with massive changes to cpu-files), doesn't change that line | 13:03 |
| stekern | still, the SI -> DI happens in that commit | 13:03 |
| stekern | so somewhere hidden in the massive cpu-file changes is something else that makes the SI go DI | 13:04 |
| juliusb | hmmm | 13:06 |
| juliusb | no, that's just the regeneration of the CGEN files | 13:06 |
| stekern | I'm speaking about 1d4d4eac0d93d514ad37db18cf04c542062d51b9 | 13:07 |
| juliusb | the source file cpu/or1korbis.cpu changed with commit 7231606b799b79b305eb43e3709e5bebf2c39bfb | 13:07 |
| juliusb | so a checkin of the source file, without the regeneration, will plant the seed for the bug, but you won't see it until later | 13:08 |
| juliusb | until the commit of the regenerated files | 13:08 |
| stekern | eeh, what tree are you looking at? | 13:08 |
| juliusb | https://github.com/pgavin/or1k-src/commits/or1k/ | 13:08 |
| juliusb | isn't that the rigt one? | 13:08 |
| stekern | yes | 13:08 |
| stekern | just that our commit # doesn't match up | 13:09 |
| juliusb | sorry, I'm being confusing aren't I? | 13:09 |
| juliusb | hehe | 13:09 |
| stekern | it's 1d4d4eac0d93d514ad37db18cf04c542062d51b9 | 13:10 |
| stekern | https://github.com/pgavin/or1k-src/commit/1d4d4eac0d93d514ad37db18cf04c542062d51b9 | 13:11 |
| stekern | =) | 13:11 |
| juliusb | basically, I think the change to the cpu/or1korbis.cpu file causing the 32-bit breakage occurs with the change to the disp26 definition (change from "sra WI (..." to sra IAI (...") | 13:11 |
| stekern | I regenerated the cpu files in that commit too, and SI went DI, so it's that one for sure | 13:11 |
| stekern | no, because sra WI is still in that commit | 13:12 |
| juliusb | hmmm | 13:12 |
| stekern | and SI goes DI there | 13:12 |
| juliusb | so you took the tree back to 1d4d4... and regnerated the opcodes files and the bug manifested itself there? | 13:13 |
| stekern | spot on | 13:13 |
| juliusb | but... not in the one before it? | 13:14 |
| stekern | right | 13:14 |
| juliusb | Ok, thanks for walking me through this :) I get it now | 13:15 |
| juliusb | could it be the case that parts of the tree weren't committed but were used to regenerated the opcodes stuff? | 13:16 |
| juliusb | but... no, if you get the exact same result, ie. the SI->DI in opcodes/or1k-ibld.c at that stage too, then it must just be something in the tree there causing that | 13:17 |
| juliusb | in the tree that is committed | 13:18 |
| stekern | I'll do a sanity check and regenerate on the commit before 1d4d4 again | 13:18 |
| stekern | yup, still SI on that one | 13:23 |
| juliusb | So the change of the df f-disp26 encode thing of WI to IAI in 7231606b7... isn't at all relevant. The bug was there whether that used WI or IAI, but the fix is to change IAI back to SI | 13:23 |
| juliusb | what happens if you change WI to SI for f-disp26 encode with the code at commit 1d4d4...?\ | 13:24 |
| juliusb | it'd surely stick to the SI? | 13:24 |
| juliusb | so... therefore it's definitely something else which effected it there | 13:24 |
| juliusb | and it _must_ have been something which is in the tree there because you're seeing the change, too | 13:25 |
| juliusb | Can I be annoying and ask you to regenerate the opcodes files at commit state 1d4d4... with removing the (MACH ORBIS-MACHS) from the PCREL-ADDR line? | 13:27 |
| juliusb | leaving everything else as it was at that state, and seeing if the DI->SI change occurs | 13:27 |
| juliusb | ... or maybe you've already tried that? | 13:27 |
| juliusb | That's the only major change I can see in the f-disp26 definition | 13:28 |
| stekern | yes, I tried removing the MACH ORBIS-MACS, no effect | 13:28 |
| juliusb | also, the PC hardware definition changes | 13:28 |
| juliusb | in the 1d4d4... change | 13:28 |
| stekern | yup, no effect | 13:29 |
| juliusb | but you tried that yesterday didn't you? | 13:29 |
| juliusb | I see the ORBIS-MACHS definition is or32,or64 | 13:29 |
| juliusb | +(define-pmacro ORBIS-MACHS or32,or64) | 13:29 |
| stekern | and changing WI to SI gives SI back from DI | 13:29 |
| juliusb | what happens if you remove or64 there | 13:29 |
| juliusb | Hmm, there's also this change: | 13:31 |
| juliusb | -(dsh h-delay-insn "delay insn addr" () (register SI)) | 13:31 |
| stekern | let's see | 13:31 |
| juliusb | +(dsh h-delay-pc "delay insn addr" () (register IAI)) | 13:31 |
| juliusb | which is introducing the IAI version to the delay slot | 13:31 |
| juliusb | but I don't really get the dsh definition thing, just reading up on it | 13:31 |
| stekern | mmm, let's try that next | 13:31 |
| juliusb | but yeah I'd try removing or64 to see if it has any effect | 13:32 |
| stekern | we're great at shootgun debugging! =) | 13:32 |
| juliusb | :) | 13:32 |
| juliusb | well it _must_ be in this commit | 13:32 |
| juliusb | I'd try some of the more obvious ones, hopefully it's not too long to do right, if I recall, running opcode generation wasn't too long | 13:33 |
| stekern | ok, removing or64 didn't make DI->SI happen | 13:33 |
| stekern | yeah, it doesn't take long, except I'm on the slow 32-bit machine now | 13:34 |
| stekern | =) | 13:34 |
| juliusb | huh? the bug was SI->DI, you mean it didn't fix it? | 13:34 |
| juliusb | It didn't reverse the bug you mean? | 13:34 |
| stekern | I'll rephrase: removing or64 didn't have any effect | 13:34 |
| juliusb | :) | 13:34 |
| juliusb | umm, dsh is: Scalar registers with no special requirements occur frequently. Macro define-simple-hardware (with alias dsh) is identical to dnh except does not include the indices, values, or handlers | 13:35 |
| stekern | where is that dsh? | 13:35 |
| juliusb | cpu/or1kcommon.cpu | 13:36 |
| stekern | ah, or1kcommon | 13:36 |
| juliusb | 262 | 13:36 |
| juliusb | i dunno, left column says 214 for old line, and 262 for new line | 13:36 |
| juliusb | I kinda guess this won't change things, though? maybe? | 13:37 |
| juliusb | maybe there's some magical link via h-delay-pc? | 13:37 |
| juliusb | (to PC encoding ops) | 13:38 |
| juliusb | but.... I'm doubting it actually, but worth a try | 13:38 |
| juliusb | ahhh, also, l-bf flags have had changes inserting IAI as the format for PC | 13:40 |
| juliusb | errr, l-bf definitions | 13:40 |
| juliusb | in cpu/or1korbis.cpu, we see | 13:41 |
| juliusb | - (delay 1 (set pc disp26)) | 13:41 |
| juliusb | + (delay 1 (set IAI pc disp26)) | 13:41 |
| stekern | changing IAI to SI on the dsh didn't have any effect | 13:41 |
| juliusb | OK | 13:41 |
| stekern | mmm, but the SI->DI is in insert_operand, I can't imagine that it is generated from the l.bf | 13:43 |
| juliusb | yeah | 13:43 |
| stekern | is h-iaddr something magic? | 13:43 |
| juliusb | i was looking at that too, but there appears to be no change to its definition | 13:45 |
| juliusb | Isn't it possible that the l-bf definitino change is effecting it? | 13:49 |
| juliusb | like, it may fix it to change DI->SI in the opcode insertion bit | 13:49 |
| juliusb | but maybe the fact that it's also a change from SI->DI in the l-bf operand checking code or something, also makes it break | 13:49 |
| stekern | yeah, but you have that change in l-bnf and l-j too | 13:51 |
| stekern | worth a shoot still, I'm building with IAI removed on all of those | 13:51 |
| stekern | nope, no change | 13:51 |
| juliusb | :-/ | 14:01 |
| juliusb | wtf is it then!? | 14:01 |
| juliusb | hehe | 14:01 |
| stekern | we're all out of shotgun shells :/ | 14:07 |
| stekern | I'm suspecting the or64-bit changes though | 14:55 |
| stekern | looking in mode.scm | 14:55 |
| stekern | ; This is defined by the target in define-cpu:word-bitsize. | 14:55 |
| stekern | (define WI #f) | 14:55 |
| stekern | (define UWI #f) | 14:55 |
| stekern | and the IAI and AI have similiar comment | 14:55 |
| stekern | otoh, that comment make me believe we _should_ explicitly use SI on the instructions, since they are 32-bits in or64, right? | 14:59 |
| stekern | jupp, removing some more or64 stuff give me a SI | 15:04 |
| stekern | or just changing the or64 word-bitsize to 32 | 15:10 |
| stekern | I wonder if the or64 bit stuff even could work on a 32-bit machine (without changing core cgen stuff) | 15:14 |
| juliusb | ahhh | 15:39 |
| juliusb | OK, but data word is 64, insn word is still 32 | 15:39 |
| juliusb | so, not sure what that word-bitsize you mentioned before pertains to | 15:39 |
| stekern | yeah, me neither, but I guess it's still handy in some cases with the WI/UWI/IAI/AI changing bitsize according to the bitsize of the target | 20:28 |
| stekern | makes me nervous what other surprises might be lurking though... | 20:29 |
Generated by irclog2html.py 2.15.2 by Marius Gedminas - find it at mg.pov.lt!