Yeah, that seems pretty reasonable. Faults aren't a hot enough path for an extra, usually unused parameter to matter. If we make it default to Null that would probably save some editting and make things like interrupts (where the inst doesn't make sense) look a little more natural. I'll try that at some point here.
Gabe Steve Reinhardt wrote: > What about just passing the StaticInst (if applicable, i.e., on any > synchronous fault) to Fault::invoke()? I'm not sure where all it gets > called from, but I'd expect it would be available at those call sites. > It'd be a big search-and-replace to fix all the function signatures, > but it would save having to squirrel it away in the thread context w/o > changing the control flow. > > Steve > > On Sat, Aug 21, 2010 at 1:01 PM, Gabe Black <[email protected]> wrote: > >> There were a couple related problems. First, in O3's commit, setInst was >> being called with an ExtMachInst cast back to a MachInst. The cast was >> totally unnecessary so I just got rid of it. Also, as you pointed out, >> MachInsts were being returned and stored, etc. where really ExtMachInsts >> should be to avoid loosing data. The next problem that introduced was >> that the ThreadState class was serializing the MachInst set with >> setInst. X86's MachInst is just a normal uint64_t, I think, so it could >> be serialized without any problems. Now that it was an ExtMachInst, >> though, SERIALIZE_SCALAR and UNSERIALIZE_SCALAR didn't work. To fix that >> I just had to add definitions for paramOut and paramIn. Note that I >> haven't actually tested that since I still don't know how to use that >> stuff and we don't have regressions for it, but there wasn't anything >> that tricky going on so it should basically just work. >> >> What I'd hoped to do initially was to eliminate the getInst/setInst pair >> entirely since those get plumbed all over the place and they're used by >> exactly one ISA in one type of fault. It also saps a bit of performance >> because that state in the ThreadState (or wherever keeps track of it) >> constantly needs to be updated even though it's rarely (or never) used. >> I didn't have enough information to try to factor it out, though, so I >> went ahead and made it work. Would it make sense to pass translation >> faults back to instructions so they have a chance to do any special >> handling that might be necessary? That would subsume special handling >> for non-faulting accesses, and it would give instructions a chance to >> tack these machInst bits onto the fault before it was invoked. I'm not >> saying that's necessarily a good idea, but it seemed worth mentioning. >> >> Gabe >> >> Steve Reinhardt wrote: >> >>> So I'm not clear on where the whole problem is here (speaking o the >>> original getInst() issue), but it looks like getInst() returns a >>> MachInst and not an ExtMachInst... not sure if it matters, but I'd >>> like to see the MachInst go into the StaticInst as well as the >>> ExtMachInst, or maybe have two flavors of ExtMachInst (one for context >>> bits and one that also has predecode info) so that someday we can have >>> the decode cache bypass the predecoder. I'm not sure how relevant >>> that is here but I thought I'd throw it out. >>> >>> Also, would it help to have getInst return a pointer rather than the >>> ExtMachInst itself? I don't completely follow your bit about >>> "serialize X86's ExtMachInst structure generically, without the CPU >>> having to know it's not a built in data type". >>> >>> Steve >>> >>> On Fri, Aug 20, 2010 at 9:33 PM, Gabe Black <[email protected]> wrote: >>> >>> >>>> Yeah, I'd guessed that's what it was. I tried to implement my second >>>> approach and that didn't work out, so I tried my third approach and that >>>> did. >>>> >>>> Now I need to teach O3 how to handle variable instruction lengths and >>>> more complex microcode, and I've rediscovered O3 does its decode in >>>> fetch. It's doing that, I think, to detect unconditional branches and >>>> other really easy to decode in Alpha instructions at the time it does >>>> branch prediction so it gets a more intelligent answer. For SPARC, I put >>>> the macroop unpacking right there so all the instructions leaving decode >>>> (which is really in fetch) are actually going to get executed. The >>>> macroops are used up and then discarded without entering the rest of the >>>> system. That works when you've got a fixed size, fairly short, straight >>>> line run of microops, but it going to be a little messier for x86. I'd >>>> like to pull the decode into the actual decode stage, but then that >>>> would disrupt what Alpha is doing with these early decode bits. I >>>> haven't thought about it enough to determine if there's a problem, but >>>> if anyone wants to throw in their two cents as far as suggestions or >>>> constraints feel free. >>>> >>>> Gabe >>>> >>>> nathan binkert wrote: >>>> >>>> >>>>> Basically, when there's a translation fault, some parts of the >>>>> instruction (the opcode, and Ra) need to be stored in a status >>>>> register so that PALCODE can use it to figure out exactly what >>>>> happened in the fault. I put a file on daystrom called >>>>> /tmp/164hrm.pdf. Section 5.2.6 details the register. >>>>> >>>>> Nate >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> Hi Alpha experts. I'm working towards getting x86 to run in O3, and >>>>>> I'm running into a minor hangup having to do with MachInsts and >>>>>> ExtMachInsts that I won't bother to go into. I could nullify the problem >>>>>> and also simplify some code if I can eliminate this line in Alpha's >>>>>> fault code >>>>>> >>>>>> http://repo.m5sim.org/m5/file/c76a14014c27/src/arch/alpha/faults.cc#l152 >>>>>> >>>>>> which grabs some bits out of the MachInst and sticks them in some >>>>>> register. Could someone please tell me what those bits are? I'd like to >>>>>> see if I can figure out a different way to derive them. Failing that, >>>>>> I'm hoping to make getInst pull out the ExtMachInst directly from the >>>>>> faulting StaticInst somehow instead of it being stuffed into the >>>>>> ThreadState or similar object and retrieved later. Failing -that-, I'll >>>>>> probably need to figure out a good way serialize X86's ExtMachInst >>>>>> structure generically, without the CPU having to know it's not a built >>>>>> in data type. >>>>>> >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> m5-dev mailing list >>>>> [email protected] >>>>> http://m5sim.org/mailman/listinfo/m5-dev >>>>> >>>>> >>>>> >>>> _______________________________________________ >>>> m5-dev mailing list >>>> [email protected] >>>> http://m5sim.org/mailman/listinfo/m5-dev >>>> >>>> >>>> >>> _______________________________________________ >>> m5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >>> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> >> > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
