> On June 6, 2013, 9:49 a.m., Steve Reinhardt wrote:
> > src/cpu/o3/lsq_unit.hh, line 863
> > <http://reviews.gem5.org/r/1872/diff/2/?file=35778#file35778line863>
> >
> >     This implies that both halves of an unaligned access have to go on the 
> > same cycle... is my interpretation accurate, and if so, does this 
> > restriction make sense?  And if it does, why don't we check up front that 
> > we have two free ports before sending the first half?
> >     
> >     Offhand, I don't see any reason for this restriction.  I can't find 
> > anything in the official docs, but some googling indicates that unaligned 
> > memory accesses are not guaranteed to be atomic, and sending both halves in 
> > the same cycle doesn't guarantee atomicity anyway (maybe if both halves are 
> > in the same cache line, but definitely not otherwise).
> 
> Andreas Hansson wrote:
>     It would in fact be much better if we sent it as two separate requests 
> (from the memory system point of view). I am strongly in favour of not 
> attempting to send two things in 0 time.
> 
> Steve Reinhardt wrote:
>     It is sent as two separate requests; I don't believe the memory system 
> can tell that they're part of a single unaligned access.  I agree that this 
> is good.
>     
>     My interpretation of this code is that it is constraining these two 
> requests to be sent on the same cycle, but this is rational in that they are 
> sent over separate cache ports.  The constraint doesn't make sense to me 
> though, particularly since this implies that an x86 core with only one cache 
> port would probably deadlock if it issued an unaligned access.
> 
> Ali Saidi wrote:
>     I agree too. I think the reason the code is this way was unaligned 
> support was hacked in. My impression is that most of the reason for the 
> problem is that at decode time you might not know the address that is being 
> loaded/stored so with the model we have of generating instructions it's not 
> clear if one or two instructions should be created.
> 
> Erik Tomusk wrote:
>     The problem is that there isn't an elegant way of enforcing a cache ports 
> limit here without refactoring the code that handles split accesses.
>     
>     Is there consensus on whether split accesses are currently handled 
> correctly or not? If not, is there a consensus on how they should be handled? 
> If so, should those changes also be part of this patch?

Sorry for the long delay, this one seemed to slip through...

I don't know what the consensus is or how these sort of split accesses should 
be handled. I guess it's no worse than the previous code, so I doesn't need to 
be part of this patch. 

My biggest concern is I can't really get my head around why the CPU needs to 
squash a bunch of instructions when one of the cache ports blocks. Did you ever 
look at Steve's comment above. Is it the case that the squashing only happens 
in the second case?


Sorry again about forgetting about this patch for so long and thanks for 
posting it,

Ali


- Ali


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1872/#review4395
-----------------------------------------------------------


On June 5, 2013, 6:13 a.m., Erik Tomusk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1872/
> -----------------------------------------------------------
> 
> (Updated June 5, 2013, 6:13 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 9722:7026fe0f45b4
> ---------------------------
> O3CPU: Revive cachePorts per-cycle dcache access limit
> This is a stop-gap patch to place a limit on the number of dcache requests the
> LSQUnit sends each cycle. Currently, the LSQUnit will send any number of
> requests, leading to unrealistic dcache usage. Note that there is an LSQUnit
> for each hardware thread, so the cachePorts limit is enforced on a per-thread
> basis.
> 
> What this patch does NOT do:
> *Limit icache accesses
> *Limit dcache accesses from sources other than the LSQUnit (e.g. accesses from
> L2)
> 
> I'd like to refactor the second half of LSQUnit<Impl>::read(), as it's very
> messy. It would be helpful to get feedback on whether what it does is
> functionally correct before I do.
> 
> It would also be helpful if someone who understands split memory accesses
> could check if that bit of code is correct, since I don't know how to test
> it.
> 
> 
> Diffs
> -----
> 
>   src/cpu/o3/O3CPU.py eb075b2b925a 
>   src/cpu/o3/iew_impl.hh eb075b2b925a 
>   src/cpu/o3/lsq_unit.hh eb075b2b925a 
>   src/cpu/o3/lsq_unit_impl.hh eb075b2b925a 
> 
> Diff: http://reviews.gem5.org/r/1872/diff/
> 
> 
> Testing
> -------
> 
> When cachePorts is set to 200 (the old value), this patch passes
> ARM/tests/fast/long with the exception that the regression complains about
> the new statistic.
> 
> 
> Thanks,
> 
> Erik Tomusk
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to