Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/40957 )
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 <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Boris Shingarov <shinga...@gmail.com>
---
M src/arch/mips/mt.hh
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Boris Shingarov: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
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);
11 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40957
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I30d0a47992750adf78c8aa0c28217da187e0cbda
Gerrit-Change-Number: 40957
Gerrit-PatchSet: 13
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Boris Shingarov <shinga...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
