#680: Add kifneg function -----------------------------------+---------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Keywords: | -----------------------------------+---------------------------------------- Add an kifneg function that does the opposite of kifpos, because kifneg is more natural (in the same sense as "ifnotzero" is natural in C.)
Incidentally, it should probably be renamed "ifmsb" instead, because it can also be applied to unsigned quantities which don't have a sign bit.
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: new => review
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by knarf):
This patch changes the definition of k8ifpos in file vectors-8-SSE2.h:240. Is this intended? Otherwise the patch looks ok (without testing it).
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by barry.wardell):
How about adding your suggested ifmsb and implementing ifpos and ifneg in terms of that?
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by eschnett):
ifmsb is implemented (just called ifneg), ifpos is implemented in terms of ifneg.
Also, the definition of k8ipos is not changed -- the routine body has been changed, and the routine name has been changed from ifpos to ifneg, so the net change is neutral.
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by knarf):
If I look at line 240, that's inside a definition of k8ifpos, and that name is not changed. However, the entire block is inside an 'elif 0' - so it should not have an effect in the end. Doesn't this change the definition of k8ifpos inside that block? Could we remove that block at some point anyway, since it will never get executed?
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by barry.wardell):
Replying to [comment:4 eschnett]:
ifmsb is implemented (just called ifneg), ifpos is implemented in terms
of ifneg.
Yes, I saw that. My suggestion is to go ahead with renaming it to ifmsb and then add the trivial definition of ifneg = ifmsb. This would have the advantage that it would mean that only ifmsb would need to be implemented in each of the vectors-*.h files and ifpos/ifneg could be implemented just once in vectors.h
Replying to [comment:5 knarf]:
If I look at line 240, that's inside a definition of k8ifpos, and that
name is not changed. However, the entire block is inside an 'elif 0' - so it should not have an effect in the end. Doesn't this change the definition of k8ifpos inside that block? Could we remove that block at some point anyway, since it will never get executed?
That's how it looks to me too. Is there a reason to keep this if 0'd code around? If there is, then I think the unintentional change in the definition of k8ifpos should be corrected.
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by eschnett):
Only implementing kifmsb, and defining kifpos and kifneg at a higher level, are a good idea.
The #if 0 code is there because it is not always clear which code executes fastest. This also depends on the compiler. I'd like to keep this (slower, but still used-to-be-correct) variant around, augmented by a respective comment.
#680: LSUThorns/Vectors: Add kifneg function ------------------------------------+--------------------------------------- Reporter: eschnett | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: review => closed * resolution: => fixed
Comment:
Applied
trac@lists.einsteintoolkit.org