Attention is currently required from: Boris Shingarov.
Hello kokoro, Boris Shingarov,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/43231
to review the following change.
Change subject: arch-mips: Fix a bug in the MIPS yield instruction.
......................................................................
arch-mips: Fix a bug in the MIPS yield instruction.
The yieldThread function implements MIPS's yield instruction, and had a
if condition in it, (src_reg && !yield_mask != 0), which upset clang. When
originally committed, this check read (src_reg & !yield_mask != 0), but
apparently as part of a cleanup sweep a long time ago, it was assumed
that the & was being used as a logical operator and was turned into &&.
Reading the actual description of what the yield instruction is supposed
to do, if src_reg is positive (it is at this point in the function),
then it's supposed to be treated as a bitvector. The YQMask register,
what gets passed in as yield_mask, can have bits set in it which mask
bits that might be set in src_reg, and if any are still set, the an
interrupt should happen, as implemented by the body of the if.
From this description, it's apparent that what the original code was
*trying* to do was to use yield_mask to mask any set bits in src_reg,
and then if any bits were left go into the body. The original author
used ! as a bitwise negating operator since what they *wanted* to do was
to block any bits in src_reg where yield_mask *is* set, and let through
any where yield_mask *is not* set. The & would do that, but only with a
bitwise negated yield_mask. Hence:
if ((src_reg & ~yield_mask) != 0) {
...
}
Change-Id: I30d0a47992750adf78c8aa0c28217da187e0cbda
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40957
Maintainer: Gabe Black <[email protected]>
Tested-by: kokoro <[email protected]>
Reviewed-by: Boris Shingarov <[email protected]>
---
M src/arch/mips/mt.hh
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/arch/mips/mt.hh b/src/arch/mips/mt.hh
index 9ab3290..da4c51a 100644
--- a/src/arch/mips/mt.hh
+++ b/src/arch/mips/mt.hh
@@ -273,7 +273,7 @@
curTick(), tc->threadId());
}
} else if (src_reg > 0) {
- if (src_reg && !yield_mask != 0) {
+ if ((src_reg & ~yield_mask) != 0) {
VPEControlReg vpeControl =
tc->readMiscReg(MISCREG_VPE_CONTROL);
vpeControl.excpt = 2;
tc->setMiscReg(MISCREG_VPE_CONTROL, vpeControl);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43231
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-0
Gerrit-Change-Id: I30d0a47992750adf78c8aa0c28217da187e0cbda
Gerrit-Change-Number: 43231
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Yuen <[email protected]>
Gerrit-Reviewer: Boris Shingarov <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Gabe Black <[email protected]>
Gerrit-Attention: Boris Shingarov <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
