Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
Am 19.03.2012 14:08, schrieb Stefan Weil: Am 19.03.2012 13:43, schrieb Andreas Färber: Am 19.03.2012 13:31, schrieb Stefan Weil: Am 19.03.2012 13:17, schrieb Stefan Hajnoczi: On Mon, Mar 19, 2012 at 10:33 AM, Andreas Färber afaer...@suse.de wrote: Am 17.03.2012 13:00, schrieb Stefan Weil: Function set_HILO is not needed anywhere. Does this cause any warnings? Given the state mips is currently in (TCG patches queuing), I'd suggest to hold this off for a bit, but I don't really mind either way. Commit message does not mention if this was never used in the first place or became unused during TCG conversion or some other refactoring? It doesn't cause any warning on my build here, so there's no strict need for this patch. I have dropped the patch for now. Please resend if you want to get it in and address Andreas' questions. the function was never used. It should be removed just to keep the code clean and free of unneeded functions. I noticed this function when I looked after the functions which follow (set_HIT0_LO, ...). Those functions are very similar, so I think set_HILO was the copy master for those functions (maybe used in a local code version whic was never committed). Could you please check that it is not used by Richard Sandiford's, Khansa Butt's and Jia Liu's patches? Then I'll happily ack. I looked for set_HI_LO in my local qemu-devel mailings from 2011 and 2012 and found only my own patches. You can also search the QEMU mailing list archive for set_HILO. Try this URL: http://lists.nongnu.org/archive/cgi-bin/namazu.cgi?query=*set_HI_LO*submit=Search!idxname=qemu-develmax=20result=normalsort=score It also shows no patches which include 'set_HILO' from the people you mentioned. This patch conflicts with the new one, but it was also sent by me and I knew that I'd have to rebase it: http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg00447.html. It's this patch which made me aware that set_HILO is unused. All that code was added for the V54xx core family and is only used for V54xx (or not used at all). Static inline functions never create a gcc warning when they are unused, as far as I know. Maybe other tools like static code analysers raise a warning. Sure, I was thinking of the static analysis tools you occasionally posted patches for. Andreas Actually, they did complain, but I did not care then: cppcheck-20110721.log:qemu/target-mips/op_helper.c:1: style: The function 'set_HILO' is never used My cppcheck-20111029.log lists 1372 functions which are never used in QEMU. Many of those are false positives (for examples caused by the way how QEMU creates helper function names using macros), so I did not care. Cheers, Stefan Are there any questions remaining? I know that a lot of MIPS related patches are waiting for commit, but maybe at least this really trivial one can finally find its way through qemu-trivial. Cheers, Stefan
Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
Hello Stefan,
Am 17.03.2012 13:00, schrieb Stefan Weil:
Function set_HILO is not needed anywhere.
Does this cause any warnings? Given the state mips is currently in (TCG
patches queuing), I'd suggest to hold this off for a bit, but I don't
really mind either way.
Commit message does not mention if this was never used in the first
place or became unused during TCG conversion or some other refactoring?
Andreas
Signed-off-by: Stefan Weil s...@weilnetz.de
---
target-mips/op_helper.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 3a20731..7b77d5a 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -193,12 +193,6 @@ static inline uint64_t get_HILO (void)
return ((uint64_t)(env-active_tc.HI[0]) 32) |
(uint32_t)env-active_tc.LO[0];
}
-static inline void set_HILO (uint64_t HILO)
-{
-env-active_tc.LO[0] = (int32_t)HILO;
-env-active_tc.HI[0] = (int32_t)(HILO 32);
-}
-
static inline void set_HIT0_LO (target_ulong arg1, uint64_t HILO)
{
env-active_tc.LO[0] = (int32_t)(HILO 0x);
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
On Mon, Mar 19, 2012 at 10:33 AM, Andreas Färber afaer...@suse.de wrote: Hello Stefan, Am 17.03.2012 13:00, schrieb Stefan Weil: Function set_HILO is not needed anywhere. Does this cause any warnings? Given the state mips is currently in (TCG patches queuing), I'd suggest to hold this off for a bit, but I don't really mind either way. Commit message does not mention if this was never used in the first place or became unused during TCG conversion or some other refactoring? It doesn't cause any warning on my build here, so there's no strict need for this patch. I have dropped the patch for now. Please resend if you want to get it in and address Andreas' questions. Thanks, Stefan
Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
Am 19.03.2012 13:17, schrieb Stefan Hajnoczi: On Mon, Mar 19, 2012 at 10:33 AM, Andreas Färber afaer...@suse.de wrote: Hello Stefan, Am 17.03.2012 13:00, schrieb Stefan Weil: Function set_HILO is not needed anywhere. Does this cause any warnings? Given the state mips is currently in (TCG patches queuing), I'd suggest to hold this off for a bit, but I don't really mind either way. Commit message does not mention if this was never used in the first place or became unused during TCG conversion or some other refactoring? It doesn't cause any warning on my build here, so there's no strict need for this patch. I have dropped the patch for now. Please resend if you want to get it in and address Andreas' questions. Thanks, Stefan Hi Andreas, hi Stefan the function was never used. It should be removed just to keep the code clean and free of unneeded functions. I noticed this function when I looked after the functions which follow (set_HIT0_LO, ...). Those functions are very similar, so I think set_HILO was the copy master for those functions (maybe used in a local code version whic was never committed). Static inline functions never create a gcc warning when they are unused, as far as I know. Maybe other tools like static code analysers raise a warning. Cheers, Stefan
Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
Am 19.03.2012 13:31, schrieb Stefan Weil: Am 19.03.2012 13:17, schrieb Stefan Hajnoczi: On Mon, Mar 19, 2012 at 10:33 AM, Andreas Färber afaer...@suse.de wrote: Am 17.03.2012 13:00, schrieb Stefan Weil: Function set_HILO is not needed anywhere. Does this cause any warnings? Given the state mips is currently in (TCG patches queuing), I'd suggest to hold this off for a bit, but I don't really mind either way. Commit message does not mention if this was never used in the first place or became unused during TCG conversion or some other refactoring? It doesn't cause any warning on my build here, so there's no strict need for this patch. I have dropped the patch for now. Please resend if you want to get it in and address Andreas' questions. the function was never used. It should be removed just to keep the code clean and free of unneeded functions. I noticed this function when I looked after the functions which follow (set_HIT0_LO, ...). Those functions are very similar, so I think set_HILO was the copy master for those functions (maybe used in a local code version whic was never committed). Could you please check that it is not used by Richard Sandiford's, Khansa Butt's and Jia Liu's patches? Then I'll happily ack. Static inline functions never create a gcc warning when they are unused, as far as I know. Maybe other tools like static code analysers raise a warning. Sure, I was thinking of the static analysis tools you occasionally posted patches for. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] target-mips: Remove unused inline function
Am 19.03.2012 13:43, schrieb Andreas Färber: Am 19.03.2012 13:31, schrieb Stefan Weil: Am 19.03.2012 13:17, schrieb Stefan Hajnoczi: On Mon, Mar 19, 2012 at 10:33 AM, Andreas Färber afaer...@suse.de wrote: Am 17.03.2012 13:00, schrieb Stefan Weil: Function set_HILO is not needed anywhere. Does this cause any warnings? Given the state mips is currently in (TCG patches queuing), I'd suggest to hold this off for a bit, but I don't really mind either way. Commit message does not mention if this was never used in the first place or became unused during TCG conversion or some other refactoring? It doesn't cause any warning on my build here, so there's no strict need for this patch. I have dropped the patch for now. Please resend if you want to get it in and address Andreas' questions. the function was never used. It should be removed just to keep the code clean and free of unneeded functions. I noticed this function when I looked after the functions which follow (set_HIT0_LO, ...). Those functions are very similar, so I think set_HILO was the copy master for those functions (maybe used in a local code version whic was never committed). Could you please check that it is not used by Richard Sandiford's, Khansa Butt's and Jia Liu's patches? Then I'll happily ack. I looked for set_HI_LO in my local qemu-devel mailings from 2011 and 2012 and found only my own patches. You can also search the QEMU mailing list archive for set_HILO. Try this URL: http://lists.nongnu.org/archive/cgi-bin/namazu.cgi?query=*set_HI_LO*submit=Search!idxname=qemu-develmax=20result=normalsort=score It also shows no patches which include 'set_HILO' from the people you mentioned. This patch conflicts with the new one, but it was also sent by me and I knew that I'd have to rebase it: http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg00447.html. It's this patch which made me aware that set_HILO is unused. All that code was added for the V54xx core family and is only used for V54xx (or not used at all). Static inline functions never create a gcc warning when they are unused, as far as I know. Maybe other tools like static code analysers raise a warning. Sure, I was thinking of the static analysis tools you occasionally posted patches for. Andreas Actually, they did complain, but I did not care then: cppcheck-20110721.log:qemu/target-mips/op_helper.c:1: style: The function 'set_HILO' is never used My cppcheck-20111029.log lists 1372 functions which are never used in QEMU. Many of those are false positives (for examples caused by the way how QEMU creates helper function names using macros), so I did not care. Cheers, Stefan
[Qemu-devel] [PATCH] target-mips: Remove unused inline function
Function set_HILO is not needed anywhere.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
target-mips/op_helper.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 3a20731..7b77d5a 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -193,12 +193,6 @@ static inline uint64_t get_HILO (void)
return ((uint64_t)(env-active_tc.HI[0]) 32) |
(uint32_t)env-active_tc.LO[0];
}
-static inline void set_HILO (uint64_t HILO)
-{
-env-active_tc.LO[0] = (int32_t)HILO;
-env-active_tc.HI[0] = (int32_t)(HILO 32);
-}
-
static inline void set_HIT0_LO (target_ulong arg1, uint64_t HILO)
{
env-active_tc.LO[0] = (int32_t)(HILO 0x);
--
1.7.9
