date:20170124

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

2017-01-24 Thread Gabe Black


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

Ship it!


Ship It!

- Gabe Black


On Jan. 23, 2017, 3:39 p.m., Santi Galan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3790/
> ---
> 
> (Updated Jan. 23, 2017, 3:39 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11801:80af6a1e6beb
> \-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-
> x86: fixed branching() computation for branch uops that only changes nupc and 
> not npc
> 
> When a branch micro-op belongs to a flow and the micro-op does not change the 
> nPC and just updates the nuPC
> (like a 'rep movs' flow), `branching()` function always returns not-taken no 
> matter  actual
> micro-branch outcome. Provided fix adds to the equation  nuPC attribute 
> checking since these kind of branch
> micro-op only updates that pointer.
> 
> This issue has been found while debugging the performance of a copy-loop 
> implemented with `memcopy` function. Without the fix, 'rep movss' internal 
> micro-branch was always predicted as not-taken causing an squash event after 
> every branch micro-branch execution.
> 
> Using the provided test, branch mispredition went from *1922* without the fix 
> to *7*.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/types.hh e47703369039 
> 
> Diff: http://reviews.gem5.org/r/3790/diff/
> 
> 
> Testing
> ---
> 
> I used this command line to evaluate the performance:
> 
> ```
> ./build/X86/gem5.opt configs/example/se.py --cpu-type=detailed --caches 
> --l2cache -c /path/to/binary
> ```
> 
> This is the source code of the binary:
> ```
> #include 
> #include "m5op.h"
> 
> #define SIZE 15*1024
> 
> //arrays are cache aligned
> char a [SIZE] __attribute__((aligned(0x40)));
> char b [SIZE] __attribute__((aligned(0x40)));
> 
> int main ()
> {
> //some warmup
> int i;
> for (i = 0; i < SIZE; ++i)
> {
> a[i] = 1;
> b[i] = 2;
> }
> 
> m5_reset_stats(0, 0);
> memcpy(a, b, SIZE);
> m5_exit(0);
> 
> //keep compiler happy
> return 0;
> }
> ```
> 
> Which was compiled with this makefile:
> 
> ```
> GEM5_PATH=/path/to/gem5
> 
> binary: binary.c $(GEM5_PATH)/util/m5/m5op_x86.S
>   gcc -o $@ $^ -static -I$(GEM5_PATH)/util/m5
> ```
> 
> 
> Thanks,
> 
> Santi Galan
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3773: ruby: PerfectSwitch add assured access arbitration

2017-01-24 Thread Joel Hestness



> On Jan. 19, 2017, 9 p.m., Brad Beckmann wrote:
> > Overall this patch looks really good.  I'm sure it helps out GPU 
> > simulations quite a bit.  I do have a few questions/comments I would like 
> > answered/addressed before I give it a ship it.

Agreed on your suggestions. I've updated the patch.


- Joel


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


On Jan. 25, 2017, 6:49 a.m., Joel Hestness wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3773/
> ---
> 
> (Updated Jan. 25, 2017, 6:49 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11802:ca5c5b982ea5
> ---
> ruby: PerfectSwitch add assured access arbitration
> 
> When operating near bandwidth saturation and using finite cache hierarchy
> buffering, the round-robin arbitration in the PerfectSwitch caused low ID
> input buffers to gain access to the switch more frequently than other input
> buffers that might contain requests. This resulted from the priority cycling
> starting on input buffers with no pending requests and cycling around to the
> low ID buffers with pending requests. Part of the problem was that
> input-to-output port allocation was done on-the-fly while cycling through
> input ports.
> 
> To fix this, refactor the PerfectSwitch to remove on-the-fly arbitration, and
> better delineate port allocation from switch traversal. Then, implement
> cycling-priority assured access arbitration using output port request batches
> to ensure that all input ports are given the same priority when buffers are
> full.
> 
> This fix reduces GPU core progress asymmetry from >3x down to <12%, and in
> line with hardware.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/simple/PerfectSwitch.hh cd7f3a1dbf55 
>   src/mem/ruby/network/simple/PerfectSwitch.cc cd7f3a1dbf55 
> 
> Diff: http://reviews.gem5.org/r/3773/diff/
> 
> 
> Testing
> ---
> 
> Extensive testing and use in gem5-gpu. Used GPU to saturate cache hierarchy
> bandwidth, and tracked threadblock progress to witness asymmetry. Repeated
> this testing after the fix to see greatly reduced asymmetry. Also, in these
> small tests, simulator run time improves slightly due to reduced amount of
> work performed by PerfectSwitch arbitration. Also, have run thousands of
> simulations with this patch to verify that the changes work for a wide
> range of simulated system behaviors.
> 
> 
> Thanks,
> 
> Joel Hestness
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3773: ruby: PerfectSwitch add assured access arbitration

2017-01-24 Thread Joel Hestness


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

(Updated Jan. 25, 2017, 6:49 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11802:ca5c5b982ea5
---
ruby: PerfectSwitch add assured access arbitration

When operating near bandwidth saturation and using finite cache hierarchy
buffering, the round-robin arbitration in the PerfectSwitch caused low ID
input buffers to gain access to the switch more frequently than other input
buffers that might contain requests. This resulted from the priority cycling
starting on input buffers with no pending requests and cycling around to the
low ID buffers with pending requests. Part of the problem was that
input-to-output port allocation was done on-the-fly while cycling through
input ports.

To fix this, refactor the PerfectSwitch to remove on-the-fly arbitration, and
better delineate port allocation from switch traversal. Then, implement
cycling-priority assured access arbitration using output port request batches
to ensure that all input ports are given the same priority when buffers are
full.

This fix reduces GPU core progress asymmetry from >3x down to <12%, and in
line with hardware.


Diffs (updated)
-

  src/mem/ruby/network/simple/PerfectSwitch.hh cd7f3a1dbf55 
  src/mem/ruby/network/simple/PerfectSwitch.cc cd7f3a1dbf55 

Diff: http://reviews.gem5.org/r/3773/diff/


Testing
---

Extensive testing and use in gem5-gpu. Used GPU to saturate cache hierarchy
bandwidth, and tracked threadblock progress to witness asymmetry. Repeated
this testing after the fix to see greatly reduced asymmetry. Also, in these
small tests, simulator run time improves slightly due to reduced amount of
work performed by PerfectSwitch arbitration. Also, have run thousands of
simulations with this patch to verify that the changes work for a wide
range of simulated system behaviors.


Thanks,

Joel Hestness

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Compilation error for gem5 after statfs change

2017-01-24 Thread Bjoern A. Zeeb


On 24 Jan 2017, at 22:08, Jason Lowe-Power wrote:


Hi Brandon,

I think this is a "real" bug:
http://qa.gem5.org//1905/compiling-problem-gem5-mac-os-10-11-6-scons-build-arm-gem5-opt.
I think there are a few more places that need an #ifdef NO_STATFS. 
Could

you look into it and post a patch if there's a problem? If not, please
reply to the gem5 QA post and let them know.


Can people try this one and probably get the #ifdefs right for NetBSD as 
well?  There are at least 3 different checks for that code;  if people 
don’t care about “style” I could get it right, but ..


diff -r e47703369039 src/sim/syscall_emul.hh
--- a/src/sim/syscall_emul.hh   Fri Jan 20 14:12:58 2017 -0500
+++ b/src/sim/syscall_emul.hh   Tue Jan 24 23:45:04 2017 +
@@ -70,6 +70,8 @@
 #include 
 #if (NO_STATFS == 0)
 #include 
+#else
+#include 
 #endif
 #include 
 #include 
@@ -530,20 +532,25 @@
 {
 TypedBufferArg tgt(addr);

+tgt->f_type = TheISA::htog(host->f_type);
 #if defined(__OpenBSD__) || defined(__APPLE__) || defined(__FreeBSD__)
-tgt->f_type = 0;
+tgt->f_bsize = TheISA::htog(host->f_iosize);
 #else
-tgt->f_type = TheISA::htog(host->f_type);
+tgt->f_bsize = TheISA::htog(host->f_bsize);
 #endif
-tgt->f_bsize = TheISA::htog(host->f_bsize);
 tgt->f_blocks = TheISA::htog(host->f_blocks);
 tgt->f_bfree = TheISA::htog(host->f_bfree);
 tgt->f_bavail = TheISA::htog(host->f_bavail);
 tgt->f_files = TheISA::htog(host->f_files);
 tgt->f_ffree = TheISA::htog(host->f_ffree);
 memcpy(>f_fsid, >f_fsid, sizeof(host->f_fsid));
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__FreeBSD__)
+tgt->f_namelen = TheISA::htog(host->f_namemax);
+tgt->f_frsize = TheISA::htog(host->f_bsize);
+#else
 tgt->f_namelen = TheISA::htog(host->f_namelen);
 tgt->f_frsize = TheISA::htog(host->f_frsize);
+#endif
 memcpy(>f_spare, >f_spare, sizeof(host->f_spare));

 tgt.copyOut(mem);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Bug in x86 stack instructions

2017-01-24 Thread Jason Lowe-Power

Thanks for helping me work this out.

I got the binary working by completely removing the AddrSizeFlagBit. The
only place this bit is used is in the LdStOp (it's set to true if
legacy.addr is true) and then, in the TLB, if the AddrSizeFlagBit is set it
truncates the address to 32-bits removing the upper-order bits.

By removing this flag bit, and adding "addressSize=ssz" to the st micro-op,
the binary is now working.

My next question: How can I have any confidence that this doesn't break
other things? Especially when we're talking about 32-bit compatibility
mode. Other than the hello32 binary, are there other things I should run?

My intuition says this is an OK change. The addr field in the Request
should never have the upper-order 32 bits set if the instruction is a
32-bit-mode instruction. The instruction implementation already takes care
of this, as I found with this bug.

Other thoughts? Does anyone know if it is definitely required to use
AddrSizeFlagBit to truncate the address in the TLB?

If not, I'll post a patch for this tomorrow.

Thanks,
Jason

On Tue, Jan 24, 2017 at 12:19 PM Steve Reinhardt  wrote:

> Hmm, seems like there's a little bit of an inconsistency in that the
> request is using the legacy.addr bit (which is set by the decoder when it
> sees the address size override prefix [1]) directly, while the legacy.addr
> bit is also used to calculate the addressSize value [2] but can be
> overridden (as we are doing). So if addressSize is overridden then
> AddrSizeFlagBit should no longer be set based on legacy.addr.
>
> Or another way of looking at it is that the same process of checking
> legacy.addr to override the address size is done in two places, once in the
> decoder [2] and again in the TLB [3] and it's not clear how to suppress the
> latter one.
>
> I suppose one gnarly way of doing it would be to go into the micro-op
> definition somewhere and clear the AddrSizeFlagBit out of memFlags if
> addressSize != env.addressSize (i.e., the address size was overridden).
> There's probably a cleaner way, but that's the easiest path I can think of
> (if it even works).
>
> [1] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#195
> [2] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#390
> [3] http://grok.gem5.org/source/xref/gem5/src/arch/x86/tlb.cc#315
>
>
> On Tue, Jan 24, 2017 at 8:37 AM, Jason Lowe-Power 
> wrote:
>
> > Hi Steve,
> >
> > That was super helpful. I'm now a step closer to solving this!
> >
> > Your suggestion of =ssz, lead me to search for the uses of that in x86,
> and
> > it turns out that almost all of other stack instructions have
> dataSize=ssz.
> > So, I added both dataSize=ssz and addressSize=ssz to the call
> instruction,
> > though I think only the addressSize is actually needed, but I'm not
> > certain.
> >
> > Now, the address is passed to the Request object correctly, but the
> program
> > still fails. The problem now is that the request object is getting
> > the AddrSizeFlagBit set to true, because machInst.legacy.addr is true.
> > Thus, the TLB only uses the lower 32 bits of the 64-bit address.
> >
> > Any ideas on how to change the instruction's memFlags from the macro-op
> > definition? They are set on
> > http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> > x86/isa/microops/ldstop.isa#l334
> > .
> >
> > It would be nice if we could fix this in the decoder. I think the logic
> > should be "if the address prefix is set and this is an implicit stack
> > operation, ignore the address prefix". However, I'm not sure how to tell
> if
> > the instruction is "an implicit stack operation" from the decoder.
> >
> > Thanks,
> > Jason
> >
> > On Tue, Jan 24, 2017 at 9:05 AM Steve Reinhardt 
> wrote:
> >
> > My recollection of how all this works is that the arguments to the 'st'
> > micro-op get turned into arguments to a call to the StoreOp constructor:
> >
> >597  > existrg/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l597
> >  > x86/isa/microops/ldstop.isa#l597>
> > >
> > class StoreOp(LdStOp):
> >598 <
> > http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> > x86/isa/microops/ldstop.isa#l598
> > >
> > def __init__(self, data, segment, addr, disp = 0,
> >599 <
> > http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> > x86/isa/microops/ldstop.isa#l599
> > >
> > dataSize="env.dataSize",
> >600 <
> > http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> > x86/isa/microops/ldstop.isa#l600
> > >
> > addressSize="env.addressSize",
> >601 <
> > http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> > x86/isa/microops/ldstop.isa#l601
> > >
> > atCPL0=False, nonSpec=False):
> >
> > so the "-env.dataSize" you see is actually the displacement for the
> store,
> > not the dataSize or

[gem5-dev] Compilation error for gem5 after statfs change

2017-01-24 Thread Jason Lowe-Power

Hi Brandon,

I think this is a "real" bug:
http://qa.gem5.org//1905/compiling-problem-gem5-mac-os-10-11-6-scons-build-arm-gem5-opt.
I think there are a few more places that need an #ifdef NO_STATFS. Could
you look into it and post a patch if there's a problem? If not, please
reply to the gem5 QA post and let them know.

Thanks,
Jason
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Bug in x86 stack instructions

2017-01-24 Thread Steve Reinhardt

Hmm, seems like there's a little bit of an inconsistency in that the
request is using the legacy.addr bit (which is set by the decoder when it
sees the address size override prefix [1]) directly, while the legacy.addr
bit is also used to calculate the addressSize value [2] but can be
overridden (as we are doing). So if addressSize is overridden then
AddrSizeFlagBit should no longer be set based on legacy.addr.

Or another way of looking at it is that the same process of checking
legacy.addr to override the address size is done in two places, once in the
decoder [2] and again in the TLB [3] and it's not clear how to suppress the
latter one.

I suppose one gnarly way of doing it would be to go into the micro-op
definition somewhere and clear the AddrSizeFlagBit out of memFlags if
addressSize != env.addressSize (i.e., the address size was overridden).
There's probably a cleaner way, but that's the easiest path I can think of
(if it even works).

[1] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#195
[2] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#390
[3] http://grok.gem5.org/source/xref/gem5/src/arch/x86/tlb.cc#315


On Tue, Jan 24, 2017 at 8:37 AM, Jason Lowe-Power 
wrote:

> Hi Steve,
>
> That was super helpful. I'm now a step closer to solving this!
>
> Your suggestion of =ssz, lead me to search for the uses of that in x86, and
> it turns out that almost all of other stack instructions have dataSize=ssz.
> So, I added both dataSize=ssz and addressSize=ssz to the call instruction,
> though I think only the addressSize is actually needed, but I'm not
> certain.
>
> Now, the address is passed to the Request object correctly, but the program
> still fails. The problem now is that the request object is getting
> the AddrSizeFlagBit set to true, because machInst.legacy.addr is true.
> Thus, the TLB only uses the lower 32 bits of the 64-bit address.
>
> Any ideas on how to change the instruction's memFlags from the macro-op
> definition? They are set on
> http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> x86/isa/microops/ldstop.isa#l334
> .
>
> It would be nice if we could fix this in the decoder. I think the logic
> should be "if the address prefix is set and this is an implicit stack
> operation, ignore the address prefix". However, I'm not sure how to tell if
> the instruction is "an implicit stack operation" from the decoder.
>
> Thanks,
> Jason
>
> On Tue, Jan 24, 2017 at 9:05 AM Steve Reinhardt  wrote:
>
> My recollection of how all this works is that the arguments to the 'st'
> micro-op get turned into arguments to a call to the StoreOp constructor:
>
>597  existrg/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l597
>  x86/isa/microops/ldstop.isa#l597>
> >
> class StoreOp(LdStOp):
>598 <
> http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> x86/isa/microops/ldstop.isa#l598
> >
> def __init__(self, data, segment, addr, disp = 0,
>599 <
> http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> x86/isa/microops/ldstop.isa#l599
> >
> dataSize="env.dataSize",
>600 <
> http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> x86/isa/microops/ldstop.isa#l600
> >
> addressSize="env.addressSize",
>601 <
> http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/
> x86/isa/microops/ldstop.isa#l601
> >
> atCPL0=False, nonSpec=False):
>
> so the "-env.dataSize" you see is actually the displacement for the store,
> not the dataSize or addressSize.  I think the problem is that the
> addressSize is using the env,addressSize that's calculated the "normal"
> way, i.e., including the effects of the override prefix.
>
> Poking around some more, there's an 'ssz' symbol that maps to env.stackSize
> [1] which gets used a lot in stack operations; for example, right after the
> store in CALL_NEAR_I, 'ssz' is subtracted off of the stack pointer. The
> calculation of env.stackSize seems to follow the rule you mentioned about
> being fixed at 64 bits in 64-bit mode [2, 3].
>
> So it might be sufficient to add ', addressSize=ssz' to the end of the 'st'
> micro-op. Oddly though I don't see addressSize being set on any other stack
> ops (just dataSize), so I wonder if this is a problem with other stack
> instructions or not. If so, it might be useful to have an override
> hardwired in at some lower level to check if the segment is ss and force
> the address size to be stackSize (rather than adding this extra parameter
> in a dozen different places). I wouldn't know where best to do that though,
> so the manual override seems best for now.
>
> Steve
>
> [1] http://grok.gem5.org/xref/gem5/src/arch/x86/isa/microasm.isa#107
> [2] http://grok.gem5.org/xref/gem5/src/arch/x86/isa.cc#91
> [3] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#400
>
>
>
>
> On Mon,

Re: [gem5-dev] Bug in x86 stack instructions

2017-01-24 Thread Jason Lowe-Power

Hi Steve,

That was super helpful. I'm now a step closer to solving this!

Your suggestion of =ssz, lead me to search for the uses of that in x86, and
it turns out that almost all of other stack instructions have dataSize=ssz.
So, I added both dataSize=ssz and addressSize=ssz to the call instruction,
though I think only the addressSize is actually needed, but I'm not certain.

Now, the address is passed to the Request object correctly, but the program
still fails. The problem now is that the request object is getting
the AddrSizeFlagBit set to true, because machInst.legacy.addr is true.
Thus, the TLB only uses the lower 32 bits of the 64-bit address.

Any ideas on how to change the instruction's memFlags from the macro-op
definition? They are set on
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l334
.

It would be nice if we could fix this in the decoder. I think the logic
should be "if the address prefix is set and this is an implicit stack
operation, ignore the address prefix". However, I'm not sure how to tell if
the instruction is "an implicit stack operation" from the decoder.

Thanks,
Jason

On Tue, Jan 24, 2017 at 9:05 AM Steve Reinhardt  wrote:

My recollection of how all this works is that the arguments to the 'st'
micro-op get turned into arguments to a call to the StoreOp constructor:

   597 
>
class StoreOp(LdStOp):
   598 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l598
>
def __init__(self, data, segment, addr, disp = 0,
   599 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l599
>
dataSize="env.dataSize",
   600 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l600
>
addressSize="env.addressSize",
   601 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l601
>
atCPL0=False, nonSpec=False):

so the "-env.dataSize" you see is actually the displacement for the store,
not the dataSize or addressSize.  I think the problem is that the
addressSize is using the env,addressSize that's calculated the "normal"
way, i.e., including the effects of the override prefix.

Poking around some more, there's an 'ssz' symbol that maps to env.stackSize
[1] which gets used a lot in stack operations; for example, right after the
store in CALL_NEAR_I, 'ssz' is subtracted off of the stack pointer. The
calculation of env.stackSize seems to follow the rule you mentioned about
being fixed at 64 bits in 64-bit mode [2, 3].

So it might be sufficient to add ', addressSize=ssz' to the end of the 'st'
micro-op. Oddly though I don't see addressSize being set on any other stack
ops (just dataSize), so I wonder if this is a problem with other stack
instructions or not. If so, it might be useful to have an override
hardwired in at some lower level to check if the segment is ss and force
the address size to be stackSize (rather than adding this extra parameter
in a dozen different places). I wouldn't know where best to do that though,
so the manual override seems best for now.

Steve

[1] http://grok.gem5.org/xref/gem5/src/arch/x86/isa/microasm.isa#107
[2] http://grok.gem5.org/xref/gem5/src/arch/x86/isa.cc#91
[3] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#400

On Mon, Jan 23, 2017 at 4:04 PM, Jason Lowe-Power 
wrote:

> To those of you with more x86 ISA implementation knowledge than I have:
>
> I've been working through a bug one of our users found (thanks
Sanchayan!).
> It looks like current versions of ld use the 0x67 instruction prefix
> (address size override) as an optimization instead of using a nop. See
> https://www.sourceware.org/ml/binutils/2016-05/msg00323.html.
>
> This causes the call instruction to be decoded with with the "address size
> override prefix", which is correct, in a sense. From what I can tell, this
> is passed to the call instruction via "-env.dataSize" (see call
instruction
> implementation below).
>
> def macroop CALL_NEAR_I
> {
> # Make the default data size of calls 64 bits in 64 bit mode
> .adjust_env oszIn64Override
> .function_call
>
> limm t1, imm
> rdip t7
> # Check target of call
> st t7, ss, [0, t0, rsp], "-env.dataSize"
> subi rsp, rsp, ssz
> wrip t7, t1
> };
>
> Now, the bug is, according to the x86 manual, "For instructions that
> implicitly access the stack segment (SS), the address size for stack
> accesses is determined by the D (default) bit in the stack-segment
> descriptor. In 64-bit mode, the D bit is ignored, and all stack references
> have a 64-bit address size." See
> https://support.amd.com/TechDocs/24594.pdf page
> 9.
>
> Thoughts on

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

2017-01-24 Thread Andreas Hansson

Hi Alec,

Don’t worry about it. It is the reviewboard being silly.

Andreas

On 24/01/2017, 16:34, "gem5-dev on behalf of Alec Roelke"
 wrote:

>
>
>> On Jan. 24, 2017, 9:43 a.m., Andreas Sandberg wrote:
>> > Did you remove all files from the reference output directory? In that
>>case, you'll need to add a an empty placeholder file called EMPTY in the
>>reference directories. Some VCS systems (git being one of them) don't
>>track empty directories, which would interfere with test discovery.
>
>I did add them; I'm not sure why it didn't put them into the patch.  Does
>the file need to contain text for it to work?
>
>
>- Alec
>
>
>---
>This is an automatically generated e-mail. To reply, visit:
>http://reviews.gem5.org/r/3781/#review9301
>---
>
>
>On Jan. 24, 2017, 4:33 p.m., Alec Roelke wrote:
>>
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.gem5.org/r/3781/
>> ---
>>
>> (Updated Jan. 24, 2017, 4:33 p.m.)
>>
>>
>> Review request for Default.
>>
>>
>> Repository: gem5
>>
>>
>> Description
>> ---
>>
>> Changeset 11796:1c03f7dcaa8c
>> ---
>> riscv: Remove ECALL tests from insttest
>>
>> The system calls tested in rv64i.cpp in RISC-V's insttest suite have
>> different behavior depending on the operating system and file system
>>they
>> are run on. This patch removes those tests from the regression.
>>
>> [Change deletion of ECALL test to block comment.]
>> [Restore ECALL test but remove test output to test only for completion
>> without error.]
>>
>>
>> Diffs
>> -
>>
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.ini
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.json
>> 97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simerr
>>97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simout
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/stats.txt
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.ini
>> 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.jso
>>n 97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simerr
>>97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simout
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/stats.txt
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/confi
>>g.ini 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/confi
>>g.json 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simer
>>r 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simou
>>t 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/stats
>>.txt 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.ini
>> 97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.jso
>>n 97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simerr
>>97eebddaae84
>>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simout
>>97eebddaae84
>>
>>tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/stats.txt
>>97eebddaae84
>>
>> Diff: http://reviews.gem5.org/r/3781/diff/
>>
>>
>> Testing
>> ---
>>
>>
>> Thanks,
>>
>> Alec Roelke
>>
>>
>
>___
>gem5-dev mailing list
>gem5-dev@gem5.org
>http://m5sim.org/mailman/listinfo/gem5-dev

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

2017-01-24 Thread Alec Roelke



> On Jan. 24, 2017, 9:43 a.m., Andreas Sandberg wrote:
> > Did you remove all files from the reference output directory? In that case, 
> > you'll need to add a an empty placeholder file called EMPTY in the 
> > reference directories. Some VCS systems (git being one of them) don't track 
> > empty directories, which would interfere with test discovery.

I did add them; I'm not sure why it didn't put them into the patch.  Does the 
file need to contain text for it to work?


- Alec


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


On Jan. 24, 2017, 4:33 p.m., Alec Roelke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3781/
> ---
> 
> (Updated Jan. 24, 2017, 4:33 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11796:1c03f7dcaa8c
> ---
> riscv: Remove ECALL tests from insttest
> 
> The system calls tested in rv64i.cpp in RISC-V's insttest suite have
> different behavior depending on the operating system and file system they
> are run on. This patch removes those tests from the regression.
> 
> [Change deletion of ECALL test to block comment.]
> [Restore ECALL test but remove test output to test only for completion
> without error.]
> 
> 
> Diffs
> -
> 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/stats.txt 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.ini
>  97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.json
>  97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simout 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/stats.txt 
> 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3781/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alec Roelke
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3754: cpu: Simplify the rename interface and use RegId

2017-01-24 Thread Jason Lowe-Power


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

Ship it!


Ship It!

- Jason Lowe-Power


On Jan. 16, 2017, 11:46 a.m., Rekai Gonzalez Alberquilla wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3754/
> ---
> 
> (Updated Jan. 16, 2017, 11:46 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11760:686a5d1464b6
> ---
> cpu: Simplify the rename interface and use RegId
> 
> With the hierarchical RegId there are a lot of functions that are
> redundant now.
> 
> The idea behind the simplification is that instead of having the regId,
> telling which kind of register read/write/rename/lookup/etc. and then
> the function panic_if'ing if the regId is not of the appropriate type,
> what we ahve is an interface that you give it the regId, and depending
> on the type of register, it decides which kind of register to read...
> 
> Change-Id: I7d52e9e21fc01205ae365d86921a4ceb67a57178
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/x86/insts/microregop.hh 78ef8daecd81 
>   src/arch/x86/insts/static_inst.hh 78ef8daecd81 
>   src/arch/x86/insts/static_inst.cc 78ef8daecd81 
>   src/arch/x86/isa.hh 78ef8daecd81 
>   src/arch/x86/isa/microops/limmop.isa 78ef8daecd81 
>   src/cpu/base_dyn_inst.hh 78ef8daecd81 
>   src/cpu/checker/cpu.hh 78ef8daecd81 
>   src/cpu/checker/cpu_impl.hh 78ef8daecd81 
>   src/cpu/checker/thread_context.hh 78ef8daecd81 
>   src/cpu/exec_context.hh 78ef8daecd81 
>   src/cpu/minor/dyn_inst.cc 78ef8daecd81 
>   src/cpu/minor/exec_context.hh 78ef8daecd81 
>   src/cpu/minor/scoreboard.hh 78ef8daecd81 
>   src/cpu/minor/scoreboard.cc 78ef8daecd81 
>   src/cpu/o3/comm.hh 78ef8daecd81 
>   src/cpu/o3/cpu.cc 78ef8daecd81 
>   src/cpu/o3/dyn_inst.hh 78ef8daecd81 
>   src/cpu/o3/free_list.hh 78ef8daecd81 
>   src/cpu/o3/iew_impl.hh 78ef8daecd81 
>   src/cpu/o3/inst_queue_impl.hh 78ef8daecd81 
>   src/cpu/o3/probe/elastic_trace.cc 78ef8daecd81 
>   src/cpu/o3/regfile.hh 78ef8daecd81 
>   src/cpu/o3/regfile.cc 78ef8daecd81 
>   src/cpu/o3/rename.hh 78ef8daecd81 
>   src/cpu/o3/rename_impl.hh 78ef8daecd81 
>   src/cpu/o3/rename_map.hh 78ef8daecd81 
>   src/cpu/o3/rename_map.cc 78ef8daecd81 
>   src/cpu/o3/scoreboard.hh 78ef8daecd81 
>   src/cpu/o3/thread_context.hh 78ef8daecd81 
>   src/cpu/o3/thread_context_impl.hh 78ef8daecd81 
>   src/cpu/reg_class.hh 78ef8daecd81 
>   src/cpu/reg_class.cc 78ef8daecd81 
>   src/cpu/reg_class_impl.hh PRE-CREATION 
>   src/cpu/simple/exec_context.hh 78ef8daecd81 
>   src/cpu/simple_thread.hh 78ef8daecd81 
>   src/cpu/static_inst.hh 78ef8daecd81 
>   src/cpu/thread_context.hh 78ef8daecd81 
>   src/cpu/timing_expr.cc 78ef8daecd81 
>   src/arch/alpha/isa.hh 78ef8daecd81 
>   src/arch/alpha/isa/branch.isa 78ef8daecd81 
>   src/arch/alpha/isa/fp.isa 78ef8daecd81 
>   src/arch/alpha/isa/main.isa 78ef8daecd81 
>   src/arch/arm/insts/misc.cc 78ef8daecd81 
>   src/arch/arm/isa.hh 78ef8daecd81 
>   src/arch/arm/isa.cc 78ef8daecd81 
>   src/arch/arm/isa/insts/data64.isa 78ef8daecd81 
>   src/arch/arm/isa/insts/fp.isa 78ef8daecd81 
>   src/arch/arm/isa/insts/misc.isa 78ef8daecd81 
>   src/arch/mips/isa.hh 78ef8daecd81 
>   src/arch/mips/isa/base.isa 78ef8daecd81 
>   src/arch/mips/isa/formats/int.isa 78ef8daecd81 
>   src/arch/power/insts/branch.cc 78ef8daecd81 
>   src/arch/power/insts/static_inst.cc 78ef8daecd81 
>   src/arch/power/isa.hh 78ef8daecd81 
>   src/arch/sparc/isa.hh 78ef8daecd81 
>   src/arch/sparc/isa/base.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/integerop.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/mem/util.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/priv.isa 78ef8daecd81 
>   src/arch/x86/insts/microfpop.hh 78ef8daecd81 
>   src/arch/x86/insts/microldstop.hh 78ef8daecd81 
>   src/arch/x86/insts/micromediaop.hh 78ef8daecd81 
> 
> Diff: http://reviews.gem5.org/r/3754/diff/
> 
> 
> Testing
> ---
> 
> Built in regressions passing
> 
> 
> Thanks,
> 
> Rekai Gonzalez Alberquilla
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

2017-01-24 Thread Jason Lowe-Power


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

Ship it!


Ship It!

- Jason Lowe-Power


On Jan. 23, 2017, 3:39 p.m., Santi Galan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3790/
> ---
> 
> (Updated Jan. 23, 2017, 3:39 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11801:80af6a1e6beb
> \-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-
> x86: fixed branching() computation for branch uops that only changes nupc and 
> not npc
> 
> When a branch micro-op belongs to a flow and the micro-op does not change the 
> nPC and just updates the nuPC
> (like a 'rep movs' flow), `branching()` function always returns not-taken no 
> matter  actual
> micro-branch outcome. Provided fix adds to the equation  nuPC attribute 
> checking since these kind of branch
> micro-op only updates that pointer.
> 
> This issue has been found while debugging the performance of a copy-loop 
> implemented with `memcopy` function. Without the fix, 'rep movss' internal 
> micro-branch was always predicted as not-taken causing an squash event after 
> every branch micro-branch execution.
> 
> Using the provided test, branch mispredition went from *1922* without the fix 
> to *7*.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/types.hh e47703369039 
> 
> Diff: http://reviews.gem5.org/r/3790/diff/
> 
> 
> Testing
> ---
> 
> I used this command line to evaluate the performance:
> 
> ```
> ./build/X86/gem5.opt configs/example/se.py --cpu-type=detailed --caches 
> --l2cache -c /path/to/binary
> ```
> 
> This is the source code of the binary:
> ```
> #include 
> #include "m5op.h"
> 
> #define SIZE 15*1024
> 
> //arrays are cache aligned
> char a [SIZE] __attribute__((aligned(0x40)));
> char b [SIZE] __attribute__((aligned(0x40)));
> 
> int main ()
> {
> //some warmup
> int i;
> for (i = 0; i < SIZE; ++i)
> {
> a[i] = 1;
> b[i] = 2;
> }
> 
> m5_reset_stats(0, 0);
> memcpy(a, b, SIZE);
> m5_exit(0);
> 
> //keep compiler happy
> return 0;
> }
> ```
> 
> Which was compiled with this makefile:
> 
> ```
> GEM5_PATH=/path/to/gem5
> 
> binary: binary.c $(GEM5_PATH)/util/m5/m5op_x86.S
>   gcc -o $@ $^ -static -I$(GEM5_PATH)/util/m5
> ```
> 
> 
> Thanks,
> 
> Santi Galan
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

2017-01-24 Thread Jason Lowe-Power


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

Ship it!


Ship It!

- Jason Lowe-Power


On Jan. 17, 2017, 10:04 p.m., Curtis Dunham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3785/
> ---
> 
> (Updated Jan. 17, 2017, 10:04 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> sim, kvm: make KvmVM a System parameter
> 
> A KVM VM is typically a child of the System object already, but for
> solving future issues with configuration graph resolution, the most
> logical way to keep track of this object is for it to be an actual
> parameter of the System object.
> 
> Change-Id: I965ded22203ff8667db9ca02de0042ff1c772220
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/arm/kvm/KvmGic.py 97eebddaae84 
>   src/arch/arm/kvm/gic.cc 97eebddaae84 
>   src/cpu/kvm/BaseKvmCPU.py 97eebddaae84 
>   src/cpu/kvm/KvmVM.py 97eebddaae84 
>   src/cpu/kvm/base.cc 97eebddaae84 
>   src/cpu/kvm/vm.hh 97eebddaae84 
>   src/cpu/kvm/vm.cc 97eebddaae84 
>   src/sim/System.py 97eebddaae84 
>   src/sim/system.hh 97eebddaae84 
>   src/sim/system.cc 97eebddaae84 
>   configs/example/fs.py 97eebddaae84 
>   configs/example/se.py 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3785/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Curtis Dunham
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

2017-01-24 Thread Jason Lowe-Power



> On Jan. 18, 2017, 4:02 p.m., Jason Lowe-Power wrote:
> > Is there any way you can remove the #if USE_KVM from the system.cc file? I 
> > don't have any suggestions off the top of my head, though.
> 
> Curtis Dunham wrote:
> We agree that it's not the best, but we didn't see a better solution to 
> this problem.  We are willing to entertain suggestions, though.

Yeah, sorry to ask for a change without any suggestions. I guess the real 
solution is to have cpu/kvm/vm.hh not dependent on linux/kvm.hh and include the 
KVM object in all compilations of gem5, whether or not KVM support is present 
on the host. This sounds like a major pain for a few #if's though.


- Jason


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


On Jan. 17, 2017, 10:04 p.m., Curtis Dunham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3785/
> ---
> 
> (Updated Jan. 17, 2017, 10:04 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> sim, kvm: make KvmVM a System parameter
> 
> A KVM VM is typically a child of the System object already, but for
> solving future issues with configuration graph resolution, the most
> logical way to keep track of this object is for it to be an actual
> parameter of the System object.
> 
> Change-Id: I965ded22203ff8667db9ca02de0042ff1c772220
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/arm/kvm/KvmGic.py 97eebddaae84 
>   src/arch/arm/kvm/gic.cc 97eebddaae84 
>   src/cpu/kvm/BaseKvmCPU.py 97eebddaae84 
>   src/cpu/kvm/KvmVM.py 97eebddaae84 
>   src/cpu/kvm/base.cc 97eebddaae84 
>   src/cpu/kvm/vm.hh 97eebddaae84 
>   src/cpu/kvm/vm.cc 97eebddaae84 
>   src/sim/System.py 97eebddaae84 
>   src/sim/system.hh 97eebddaae84 
>   src/sim/system.cc 97eebddaae84 
>   configs/example/fs.py 97eebddaae84 
>   configs/example/se.py 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3785/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Curtis Dunham
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Bug in x86 stack instructions

2017-01-24 Thread Steve Reinhardt

My recollection of how all this works is that the arguments to the 'st'
micro-op get turned into arguments to a call to the StoreOp constructor:

   597 

class StoreOp(LdStOp):
   598 

def __init__(self, data, segment, addr, disp = 0,
   599 

dataSize="env.dataSize",
   600 

addressSize="env.addressSize",
   601 

atCPL0=False, nonSpec=False):

so the "-env.dataSize" you see is actually the displacement for the store,
not the dataSize or addressSize.  I think the problem is that the
addressSize is using the env,addressSize that's calculated the "normal"
way, i.e., including the effects of the override prefix.

Poking around some more, there's an 'ssz' symbol that maps to env.stackSize
[1] which gets used a lot in stack operations; for example, right after the
store in CALL_NEAR_I, 'ssz' is subtracted off of the stack pointer. The
calculation of env.stackSize seems to follow the rule you mentioned about
being fixed at 64 bits in 64-bit mode [2, 3].

So it might be sufficient to add ', addressSize=ssz' to the end of the 'st'
micro-op. Oddly though I don't see addressSize being set on any other stack
ops (just dataSize), so I wonder if this is a problem with other stack
instructions or not. If so, it might be useful to have an override
hardwired in at some lower level to check if the segment is ss and force
the address size to be stackSize (rather than adding this extra parameter
in a dozen different places). I wouldn't know where best to do that though,
so the manual override seems best for now.

Steve

[1] http://grok.gem5.org/xref/gem5/src/arch/x86/isa/microasm.isa#107
[2] http://grok.gem5.org/xref/gem5/src/arch/x86/isa.cc#91
[3] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#400

On Mon, Jan 23, 2017 at 4:04 PM, Jason Lowe-Power 
wrote:

> To those of you with more x86 ISA implementation knowledge than I have:
>
> I've been working through a bug one of our users found (thanks Sanchayan!).
> It looks like current versions of ld use the 0x67 instruction prefix
> (address size override) as an optimization instead of using a nop. See
> https://www.sourceware.org/ml/binutils/2016-05/msg00323.html.
>
> This causes the call instruction to be decoded with with the "address size
> override prefix", which is correct, in a sense. From what I can tell, this
> is passed to the call instruction via "-env.dataSize" (see call instruction
> implementation below).
>
> def macroop CALL_NEAR_I
> {
> # Make the default data size of calls 64 bits in 64 bit mode
> .adjust_env oszIn64Override
> .function_call
>
> limm t1, imm
> rdip t7
> # Check target of call
> st t7, ss, [0, t0, rsp], "-env.dataSize"
> subi rsp, rsp, ssz
> wrip t7, t1
> };
>
> Now, the bug is, according to the x86 manual, "For instructions that
> implicitly access the stack segment (SS), the address size for stack
> accesses is determined by the D (default) bit in the stack-segment
> descriptor. In 64-bit mode, the D bit is ignored, and all stack references
> have a 64-bit address size." See
> https://support.amd.com/TechDocs/24594.pdf page
> 9.
>
> Thoughts on how to fix this?
>
> Thanks,
> Jason
> ___
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

2017-01-24 Thread Curtis Dunham



> On Jan. 18, 2017, 4:02 p.m., Jason Lowe-Power wrote:
> > Is there any way you can remove the #if USE_KVM from the system.cc file? I 
> > don't have any suggestions off the top of my head, though.

We agree that it's not the best, but we didn't see a better solution to this 
problem.  We are willing to entertain suggestions, though.


- Curtis


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


On Jan. 17, 2017, 10:04 p.m., Curtis Dunham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3785/
> ---
> 
> (Updated Jan. 17, 2017, 10:04 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> sim, kvm: make KvmVM a System parameter
> 
> A KVM VM is typically a child of the System object already, but for
> solving future issues with configuration graph resolution, the most
> logical way to keep track of this object is for it to be an actual
> parameter of the System object.
> 
> Change-Id: I965ded22203ff8667db9ca02de0042ff1c772220
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/arm/kvm/KvmGic.py 97eebddaae84 
>   src/arch/arm/kvm/gic.cc 97eebddaae84 
>   src/cpu/kvm/BaseKvmCPU.py 97eebddaae84 
>   src/cpu/kvm/KvmVM.py 97eebddaae84 
>   src/cpu/kvm/base.cc 97eebddaae84 
>   src/cpu/kvm/vm.hh 97eebddaae84 
>   src/cpu/kvm/vm.cc 97eebddaae84 
>   src/sim/System.py 97eebddaae84 
>   src/sim/system.hh 97eebddaae84 
>   src/sim/system.cc 97eebddaae84 
>   configs/example/fs.py 97eebddaae84 
>   configs/example/se.py 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3785/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Curtis Dunham
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3754: cpu: Simplify the rename interface and use RegId

2017-01-24 Thread Rekai Gonzalez Alberquilla



> On Jan. 19, 2017, 10:13 p.m., Jason Lowe-Power wrote:
> > src/cpu/reg_class_impl.hh, line 1
> > 
> >
> > Why is this an implemenation header file?

Because it is something that can/should be inlined, as the control has 
dependence on things that can be optimized at compile-time. On the other hand, 
as it depends on 'TheISA', having the implementation in the header file would 
make the reg_class.hh depend on the ISA, and that leads to some compile time 
issues due to cyclic dependencies. Having as separate implementation file is a 
compromise solution that keeps the dependencies to the minimum required, while 
still providing the code for inlining to have the minimum impact on performance.


- Rekai


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


On Jan. 16, 2017, 11:46 a.m., Rekai Gonzalez Alberquilla wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3754/
> ---
> 
> (Updated Jan. 16, 2017, 11:46 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11760:686a5d1464b6
> ---
> cpu: Simplify the rename interface and use RegId
> 
> With the hierarchical RegId there are a lot of functions that are
> redundant now.
> 
> The idea behind the simplification is that instead of having the regId,
> telling which kind of register read/write/rename/lookup/etc. and then
> the function panic_if'ing if the regId is not of the appropriate type,
> what we ahve is an interface that you give it the regId, and depending
> on the type of register, it decides which kind of register to read...
> 
> Change-Id: I7d52e9e21fc01205ae365d86921a4ceb67a57178
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/x86/insts/microregop.hh 78ef8daecd81 
>   src/arch/x86/insts/static_inst.hh 78ef8daecd81 
>   src/arch/x86/insts/static_inst.cc 78ef8daecd81 
>   src/arch/x86/isa.hh 78ef8daecd81 
>   src/arch/x86/isa/microops/limmop.isa 78ef8daecd81 
>   src/cpu/base_dyn_inst.hh 78ef8daecd81 
>   src/cpu/checker/cpu.hh 78ef8daecd81 
>   src/cpu/checker/cpu_impl.hh 78ef8daecd81 
>   src/cpu/checker/thread_context.hh 78ef8daecd81 
>   src/cpu/exec_context.hh 78ef8daecd81 
>   src/cpu/minor/dyn_inst.cc 78ef8daecd81 
>   src/cpu/minor/exec_context.hh 78ef8daecd81 
>   src/cpu/minor/scoreboard.hh 78ef8daecd81 
>   src/cpu/minor/scoreboard.cc 78ef8daecd81 
>   src/cpu/o3/comm.hh 78ef8daecd81 
>   src/cpu/o3/cpu.cc 78ef8daecd81 
>   src/cpu/o3/dyn_inst.hh 78ef8daecd81 
>   src/cpu/o3/free_list.hh 78ef8daecd81 
>   src/cpu/o3/iew_impl.hh 78ef8daecd81 
>   src/cpu/o3/inst_queue_impl.hh 78ef8daecd81 
>   src/cpu/o3/probe/elastic_trace.cc 78ef8daecd81 
>   src/cpu/o3/regfile.hh 78ef8daecd81 
>   src/cpu/o3/regfile.cc 78ef8daecd81 
>   src/cpu/o3/rename.hh 78ef8daecd81 
>   src/cpu/o3/rename_impl.hh 78ef8daecd81 
>   src/cpu/o3/rename_map.hh 78ef8daecd81 
>   src/cpu/o3/rename_map.cc 78ef8daecd81 
>   src/cpu/o3/scoreboard.hh 78ef8daecd81 
>   src/cpu/o3/thread_context.hh 78ef8daecd81 
>   src/cpu/o3/thread_context_impl.hh 78ef8daecd81 
>   src/cpu/reg_class.hh 78ef8daecd81 
>   src/cpu/reg_class.cc 78ef8daecd81 
>   src/cpu/reg_class_impl.hh PRE-CREATION 
>   src/cpu/simple/exec_context.hh 78ef8daecd81 
>   src/cpu/simple_thread.hh 78ef8daecd81 
>   src/cpu/static_inst.hh 78ef8daecd81 
>   src/cpu/thread_context.hh 78ef8daecd81 
>   src/cpu/timing_expr.cc 78ef8daecd81 
>   src/arch/alpha/isa.hh 78ef8daecd81 
>   src/arch/alpha/isa/branch.isa 78ef8daecd81 
>   src/arch/alpha/isa/fp.isa 78ef8daecd81 
>   src/arch/alpha/isa/main.isa 78ef8daecd81 
>   src/arch/arm/insts/misc.cc 78ef8daecd81 
>   src/arch/arm/isa.hh 78ef8daecd81 
>   src/arch/arm/isa.cc 78ef8daecd81 
>   src/arch/arm/isa/insts/data64.isa 78ef8daecd81 
>   src/arch/arm/isa/insts/fp.isa 78ef8daecd81 
>   src/arch/arm/isa/insts/misc.isa 78ef8daecd81 
>   src/arch/mips/isa.hh 78ef8daecd81 
>   src/arch/mips/isa/base.isa 78ef8daecd81 
>   src/arch/mips/isa/formats/int.isa 78ef8daecd81 
>   src/arch/power/insts/branch.cc 78ef8daecd81 
>   src/arch/power/insts/static_inst.cc 78ef8daecd81 
>   src/arch/power/isa.hh 78ef8daecd81 
>   src/arch/sparc/isa.hh 78ef8daecd81 
>   src/arch/sparc/isa/base.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/integerop.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/mem/util.isa 78ef8daecd81 
>   src/arch/sparc/isa/formats/priv.isa 78ef8daecd81 
>   src/arch/x86/insts/microfpop.hh 78ef8daecd81 
>   src/arch/x86/insts/microldstop.hh 78ef8daecd81 
>   src/arch/x86/insts/micromediaop.hh 78ef8daecd81 
> 
> Diff:

Re: [gem5-dev] Review Request 3757: arch: added generic vector register

2017-01-24 Thread Rekai Gonzalez Alberquilla



> On Jan. 20, 2017, 5:11 p.m., Tony Gutierrez wrote:
> > LGTM, nice implementation. I just have an ask for a few more comments.
> > 
> > 1) A high-level description of the API, and some direction on usage for 
> > this library would be useful as a top-level comment in the file.
> > 2) LaneData view is lacking any comments.
> > 3) How should users handle vector operations, e.g. multiplication? Should 
> > everything be handled through the views such that the primitive data types 
> > are accessed and normal operators used? Or should operators be overloaded 
> > here?

I will make sure the documentation is more comprehensive.
In the particular for 3), the designer inside me tells me that the proposed 
class should be just the container for the bits, and provide facilities to 
access bits, or parts of them, and it is up to the ISA to give semantics to 
that. Therefore, operators should not be overloaded, and it is up to the ISA 
side to use the data apropriately. By this I mean, the VecReg should understand 
things like "Think of the bits as a vector of 32bit elements, and give me the 
3rd", but it should be agnostic of whether those bits are a signed int, an 
unsigned int, a float or any other thing.

Thanks for taking the time!


- Rekai


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


On Jan. 16, 2017, 11:54 a.m., Rekai Gonzalez Alberquilla wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3757/
> ---
> 
> (Updated Jan. 16, 2017, 11:54 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11762:8b0fdbea39bf
> ---
> arch: added generic vector register
> 
> This commit adds a new generic vector register to have a cleaner
> implementation of SIMD ISAs.
> 
> Nathanael's idea, Rekai's implementation.
> 
> Change-Id: I60b250bba6423153b7e04d2e6988d517a70a3e6b
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/generic/vec_reg.hh PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3757/diff/
> 
> 
> Testing
> ---
> 
> Builtin regressions
> 
> 
> Thanks,
> 
> Rekai Gonzalez Alberquilla
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3757: arch: added generic vector register

2017-01-24 Thread Rekai Gonzalez Alberquilla



> On Jan. 19, 2017, 10:28 p.m., Jason Lowe-Power wrote:
> > Seems OK to me. Don't let anyone ever tell you that C++ makes reading code 
> > easy... I appreciate all of the compile-time tricks going on here, but it 
> > is incredibly difficult to read.
> > 
> > Maybe some more comments for each class that have an example on how to use 
> > it would be helpful? I'm a little concerned what's going to happen to this 
> > code in 5 years. Will a new person be able to come in and understand what's 
> > going on? Though, it isn't like any of this ISA code is understandable 
> > until you put weeks or months of effort in. So, feel free to ignore this 
> > comment.

I will make sure the documentation is more comprehensive. Thanks for taking the 
time!


- Rekai


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


On Jan. 16, 2017, 11:54 a.m., Rekai Gonzalez Alberquilla wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3757/
> ---
> 
> (Updated Jan. 16, 2017, 11:54 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11762:8b0fdbea39bf
> ---
> arch: added generic vector register
> 
> This commit adds a new generic vector register to have a cleaner
> implementation of SIMD ISAs.
> 
> Nathanael's idea, Rekai's implementation.
> 
> Change-Id: I60b250bba6423153b7e04d2e6988d517a70a3e6b
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   src/arch/generic/vec_reg.hh PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3757/diff/
> 
> 
> Testing
> ---
> 
> Builtin regressions
> 
> 
> Thanks,
> 
> Rekai Gonzalez Alberquilla
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

2017-01-24 Thread Andreas Sandberg


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


Did you remove all files from the reference output directory? In that case, 
you'll need to add a an empty placeholder file called EMPTY in the reference 
directories. Some VCS systems (git being one of them) don't track empty 
directories, which would interfere with test discovery.

- Andreas Sandberg


On Jan. 23, 2017, 7:36 p.m., Alec Roelke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3781/
> ---
> 
> (Updated Jan. 23, 2017, 7:36 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11796:3cec8d75cd13
> ---
> riscv: Remove ECALL tests from insttest
> 
> The system calls tested in rv64i.cpp in RISC-V's insttest suite have
> different behavior depending on the operating system and file system they
> are run on. This patch removes those tests from the regression.
> 
> [Change deletion of ECALL test to block comment.]
> [Restore ECALL test but remove test output to test only for completion
> without error.]
> 
> 
> Diffs
> -
> 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/stats.txt 
> 97eebddaae84 
>   tests/test-progs/insttest/src/riscv/rv64i.cpp 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.json
>  97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simout 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.json 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.ini
>  97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simout 
> 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3781/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alec Roelke
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

2017-01-24 Thread Santi Galan



> On gen. 24, 2017, 8:53 a.m., Santi Galan wrote:
> > Ship It!

My bad, I am not familiar with the process and I thought that the "Ship it!" 
button would change somehow the revision state.

Santi


- Santi


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


On gen. 23, 2017, 3:39 p.m., Santi Galan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3790/
> ---
> 
> (Updated gen. 23, 2017, 3:39 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11801:80af6a1e6beb
> \-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-
> x86: fixed branching() computation for branch uops that only changes nupc and 
> not npc
> 
> When a branch micro-op belongs to a flow and the micro-op does not change the 
> nPC and just updates the nuPC
> (like a 'rep movs' flow), `branching()` function always returns not-taken no 
> matter  actual
> micro-branch outcome. Provided fix adds to the equation  nuPC attribute 
> checking since these kind of branch
> micro-op only updates that pointer.
> 
> This issue has been found while debugging the performance of a copy-loop 
> implemented with `memcopy` function. Without the fix, 'rep movss' internal 
> micro-branch was always predicted as not-taken causing an squash event after 
> every branch micro-branch execution.
> 
> Using the provided test, branch mispredition went from *1922* without the fix 
> to *7*.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/types.hh e47703369039 
> 
> Diff: http://reviews.gem5.org/r/3790/diff/
> 
> 
> Testing
> ---
> 
> I used this command line to evaluate the performance:
> 
> ```
> ./build/X86/gem5.opt configs/example/se.py --cpu-type=detailed --caches 
> --l2cache -c /path/to/binary
> ```
> 
> This is the source code of the binary:
> ```
> #include 
> #include "m5op.h"
> 
> #define SIZE 15*1024
> 
> //arrays are cache aligned
> char a [SIZE] __attribute__((aligned(0x40)));
> char b [SIZE] __attribute__((aligned(0x40)));
> 
> int main ()
> {
> //some warmup
> int i;
> for (i = 0; i < SIZE; ++i)
> {
> a[i] = 1;
> b[i] = 2;
> }
> 
> m5_reset_stats(0, 0);
> memcpy(a, b, SIZE);
> m5_exit(0);
> 
> //keep compiler happy
> return 0;
> }
> ```
> 
> Which was compiled with this makefile:
> 
> ```
> GEM5_PATH=/path/to/gem5
> 
> binary: binary.c $(GEM5_PATH)/util/m5/m5op_x86.S
>   gcc -o $@ $^ -static -I$(GEM5_PATH)/util/m5
> ```
> 
> 
> Thanks,
> 
> Santi Galan
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

2017-01-24 Thread Santi Galan


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

Ship it!


Ship It!

- Santi Galan


On gen. 23, 2017, 3:39 p.m., Santi Galan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3790/
> ---
> 
> (Updated gen. 23, 2017, 3:39 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11801:80af6a1e6beb
> \-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-
> x86: fixed branching() computation for branch uops that only changes nupc and 
> not npc
> 
> When a branch micro-op belongs to a flow and the micro-op does not change the 
> nPC and just updates the nuPC
> (like a 'rep movs' flow), `branching()` function always returns not-taken no 
> matter  actual
> micro-branch outcome. Provided fix adds to the equation  nuPC attribute 
> checking since these kind of branch
> micro-op only updates that pointer.
> 
> This issue has been found while debugging the performance of a copy-loop 
> implemented with `memcopy` function. Without the fix, 'rep movss' internal 
> micro-branch was always predicted as not-taken causing an squash event after 
> every branch micro-branch execution.
> 
> Using the provided test, branch mispredition went from *1922* without the fix 
> to *7*.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/types.hh e47703369039 
> 
> Diff: http://reviews.gem5.org/r/3790/diff/
> 
> 
> Testing
> ---
> 
> I used this command line to evaluate the performance:
> 
> ```
> ./build/X86/gem5.opt configs/example/se.py --cpu-type=detailed --caches 
> --l2cache -c /path/to/binary
> ```
> 
> This is the source code of the binary:
> ```
> #include 
> #include "m5op.h"
> 
> #define SIZE 15*1024
> 
> //arrays are cache aligned
> char a [SIZE] __attribute__((aligned(0x40)));
> char b [SIZE] __attribute__((aligned(0x40)));
> 
> int main ()
> {
> //some warmup
> int i;
> for (i = 0; i < SIZE; ++i)
> {
> a[i] = 1;
> b[i] = 2;
> }
> 
> m5_reset_stats(0, 0);
> memcpy(a, b, SIZE);
> m5_exit(0);
> 
> //keep compiler happy
> return 0;
> }
> ```
> 
> Which was compiled with this makefile:
> 
> ```
> GEM5_PATH=/path/to/gem5
> 
> binary: binary.c $(GEM5_PATH)/util/m5/m5op_x86.S
>   gcc -o $@ $^ -static -I$(GEM5_PATH)/util/m5
> ```
> 
> 
> Thanks,
> 
> Santi Galan
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron <m5test@zizzer> /z/m5/regression/do-regression --scratch all

2017-01-24 Thread Andreas Hansson

Hi Joel,

We have been stalling on getting http://reviews.gem5.org/r/3781/ shipped.
Once that is done we can update the stats.

If Jason can help the patch then I am happy to bump the stats.

Andreas

On 24/01/2017, 06:09, "gem5-dev on behalf of Joel Hestness"
 wrote:

>Ping! Can someone help us out with fixing up these regressions, or do we
>have a plan for how to proceed with them?
>
>I have at least one non-trivial Ruby patch incoming, and I'm worried about
>validating its correctness.
>
>  Thanks!
>  Joel
>
>
>
>On Thu, Dec 29, 2016 at 11:44 AM, Joel Hestness 
>wrote:
>
>> Hi Arthur and Jason,
>>   I just bisected and tracked down where these regressions changed. It
>> looks like changeset 11781:1ae84c76066b
>>  on Dec. 21st
>> caused the branch predictor to change its baseline prediction accuracy,
>> causing a few of the regression tests to show changed stats.
>>
>>   Can you please let us know if you're able to update the stats to get
>> these to pass?
>>
>>   Thanks!
>>   Joel
>>
>> On Sun, Dec 25, 2016 at 5:03 PM, Cron Daemon
>>
>> wrote:
>>
>>> *
>>>build/SPARC/tests/opt/long/fs/80.solaris-boot/sparc/solaris/t1000-simple
>>>-atomic:
>>> FAILED!
>>> * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing:
>>> CHANGED!
>>> * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing:
>>> CHANGED!
>>> *
>>>build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing-mt:
>>> CHANGED!
>>> *
>>>build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3:
>>> CHANGED!
>>> *
>>>build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3-dual:
>>> CHANGED!
>>> * build/MIPS/tests/opt/quick/se/00.hello/mips/linux/o3-timing:
>>> CHANGED!
>>> * build/POWER/tests/opt/quick/se/00.hello/power/linux/o3-timing:
>>> CHANGED!
>>> * build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/o3-timing:
>>> CHANGED!
>>> *
>>>build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/o3-t
>>>iming-mp:
>>> CHANGED!
>>> * build/X86/tests/opt/quick/se/00.hello/x86/linux/o3-timing:
>>>CHANGED!
>>> *
>>>build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-minor:
>>> CHANGED!
>>> * build/X86/tests/opt/long/se/10.mcf/x86/linux/o3-timing: CHANGED!
>>> * build/X86/tests/opt/long/se/70.twolf/x86/linux/o3-timing:
>>>CHANGED!
>>> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/minor-timing:
>>> CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview-minor-dual:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/50.vortex/arm/linux/o3-timing:
>>>CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview-o3-dual:
>>> CHANGED!
>>> *
>>>build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing-checker:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/70.twolf/arm/linux/o3-timing:
>>>CHANGED!
>>> * build/ARM/tests/opt/long/se/40.perlbmk/arm/linux/o3-timing:
>>> CHANGED!
>>> * build/ARM/tests/opt/quick/se/00.hello/arm/linux/o3-timing:
>>>CHANGED!
>>> * build/ARM/tests/opt/long/se/50.vortex/arm/linux/minor-timing:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview-o3:
>>> CHANGED!
>>> * build/X86/tests/opt/long/se/20.parser/x86/linux/o3-timing:
>>>CHANGED!
>>> * build/ARM/tests/opt/long/se/30.eon/arm/linux/minor-timing:
>>>CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-minor-dua
>>>l:
>>> CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview-minor:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/10.mcf/arm/linux/o3-timing: CHANGED!
>>> * build/ARM/tests/opt/long/se/20.parser/arm/linux/o3-timing:
>>>CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-minor:
>>> CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-o3-dual:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/30.eon/arm/linux/o3-timing: CHANGED!
>>> * build/ARM/tests/opt/long/se/10.mcf/arm/linux/minor-timing:
>>>CHANGED!
>>> * build/ARM/tests/opt/long/se/20.parser/arm/linux/minor-timing:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/70.twolf/arm/linux/minor-timing:
>>> CHANGED!
>>> *
>>>build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview64-o3:
>>> CHANGED!
>>> * build/ARM/tests/opt/long/se/40.perlbmk/arm/linux/minor-timing:
>>> CHANGED!
>>> * build/RISCV/tests/opt/quick/se/00.hello/riscv/linux/minor-timing:
>>> CHANGED!
>>> *
>>>build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64m/minor-timin
>>>g:
>>> CHANGED!
>>> *
>>>build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64a/minor-timin
>>>g:
>>> CHANGED!
>>> *
>>>build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64f/minor-timin
>>>g:
>>>

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

2017-01-24 Thread Andreas Hansson


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

Ship it!


Ship It!

- Andreas Hansson


On Jan. 23, 2017, 7:36 p.m., Alec Roelke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3781/
> ---
> 
> (Updated Jan. 23, 2017, 7:36 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11796:3cec8d75cd13
> ---
> riscv: Remove ECALL tests from insttest
> 
> The system calls tested in rv64i.cpp in RISC-V's insttest suite have
> different behavior depending on the operating system and file system they
> are run on. This patch removes those tests from the regression.
> 
> [Change deletion of ECALL test to block comment.]
> [Restore ECALL test but remove test output to test only for completion
> without error.]
> 
> 
> Diffs
> -
> 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/stats.txt 
> 97eebddaae84 
>   tests/test-progs/insttest/src/riscv/rv64i.cpp 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.json
>  97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/simout 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing/config.json 
> 97eebddaae84 
>   
> tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-timing-ruby/config.ini
>  97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simout 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/stats.txt 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/simple-atomic/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.ini 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/config.json 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simerr 
> 97eebddaae84 
>   tests/quick/se/02.insttest/ref/riscv/linux-rv64i/minor-timing/simout 
> 97eebddaae84 
> 
> Diff: http://reviews.gem5.org/r/3781/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alec Roelke
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

Re: [gem5-dev] Review Request 3773: ruby: PerfectSwitch add assured access arbitration

Re: [gem5-dev] Review Request 3773: ruby: PerfectSwitch add assured access arbitration

Re: [gem5-dev] Compilation error for gem5 after statfs change

Re: [gem5-dev] Bug in x86 stack instructions

[gem5-dev] Compilation error for gem5 after statfs change

Re: [gem5-dev] Bug in x86 stack instructions

Re: [gem5-dev] Bug in x86 stack instructions

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

Re: [gem5-dev] Review Request 3754: cpu: Simplify the rename interface and use RegId

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

Re: [gem5-dev] Bug in x86 stack instructions

Re: [gem5-dev] Review Request 3785: sim, kvm: make KvmVM a System parameter

Re: [gem5-dev] Review Request 3754: cpu: Simplify the rename interface and use RegId

Re: [gem5-dev] Review Request 3757: arch: added generic vector register

Re: [gem5-dev] Review Request 3757: arch: added generic vector register

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

Re: [gem5-dev] Review Request 3790: x86: fixed branching computation for branch uops that only changes nupc and not npc

Re: [gem5-dev] Cron <m5test@zizzer> /z/m5/regression/do-regression --scratch all

Re: [gem5-dev] Review Request 3781: riscv: Remove ECALL tests from insttest

24 matches

Site Navigation

Mail list logo

Footer information