0

I've put together a simple macro that is meant to find the minimum of three numbers. In it's current state the macro will only work if the first or third parameter is the min, but not the second. Is there something off with my loops or comparisons that i'm not catching? Or anything I should be doing differently? Any recommendations or suggestions are appreciated.

findMin     MACRO   arg1, arg2, arg3
                
            mov eax, arg1 
            
            cmp eax, arg2         ; if eax is greater than arg2 swap
            jg swap1

            cmp eax, arg3         ; if eax is greater than arg3 swap
            jg swap2

            jmp endMac

            swap1: xchg eax, arg2
            swap2: xchg eax, arg3
            endMac:
          
            ENDM
subprimeloads
  • 372
  • 3
  • 13
  • Hi, interesting, perhaps this might be of interest https://stackoverflow.com/questions/2039730/fastest-way-to-find-out-minimum-of-3-numbers – IronMan Oct 28 '20 at 21:04
  • If arg1 > arg2, then you never even check arg3, you just return it. – prl Oct 28 '20 at 21:35
  • This macro changes the values of arg2 and arg3 in some cases. That seems like a bug, unless it is an intended side effect that you didn’t mention. Just use mov, instead of of xchg. – prl Oct 28 '20 at 21:38
  • I guess this is MASM? Macro syntax varies between assemblers, so please use a tag for the assembler you are using. – Nate Eldredge Oct 28 '20 at 22:03
  • Note that everything will break if you use this macro more than once in the same source file, because you'll have defined multiple labels named `swap1`, swap2`, etc. I'm not sure whether MASM has a local label feature, but if so you need to use it. – Nate Eldredge Oct 28 '20 at 22:04
  • 1
    Alternatively, rewrite the macro using conditional move instructions (CMOVcc). It'll be more efficient and you won't need any labels or jumps at all. – Nate Eldredge Oct 28 '20 at 22:06

2 Answers2

1

Simplified Macro to Find the Min of 3 Numbers (Min will be in EAX register)

findMin    MACRO   arg1, arg2, arg3

           LOCAL L2, L3

           mov   eax, arg1
           
           cmp   eax, arg2
           jg    l2
           cmp   eax, arg3
           jg    l3
           jmp   endMac

           L2: xchg eax, arg2
               cmp  eax, arg3
               jg   L3
               jmp  endMac
           L3: xchg eax, arg3

           endMac:

           ENDM
subprimeloads
  • 372
  • 3
  • 13
  • 1
    This breaks if any of the 3 input args were already in EAX, EDX, or ECX, and one overwrites another before you read any. It would make more sense to just `mov eax, arg2` but then read the others from `arg1` or `arg3` twice if necessary. – Peter Cordes Oct 28 '20 at 23:21
  • Also, as comments already pointed out, `J1` and `endMac` will be defined multiple times if you use this macro more than once per file, because you didn't use macro-local labels. (Using branchless `cmovcc` would have avoided needing labels.) – Peter Cordes Oct 28 '20 at 23:23
0

The MASM macro functionality if very powerful. You can validate the types of the parameters and even do conditional assembly. Combined with the Conditional Moves CMOVcc available since the Intel P6 family processors you could optimize your procedure to

min MACRO arg1:REQ, arg2:REQ, arg3:REQ
  ; Returns result in EAX register 
  IF ((OPATTR (arg2)) AND 00000100b) or ((OPATTR (arg3)) AND 00000100b) 
    ECHO ### Function min: No constants allowed as arguments
  ELSEIF ((OPATTR (arg1)) AND 00000100b)
    ; Move constant to EAX if it's not EAX
    mov    eax, arg1  
  ELSEIF (SIZE TYPE arg1 NE 4)
    ECHO ### Function min: First parameter must be of DWORD type
  ELSEIF SIZE TYPE arg2 NE 4
    ECHO ### Function min: Second parameter must be of DWORD type
  ELSEIF SIZE TYPE arg3 NE 4
    ECHO ### Function min: Third parameter must be of DWORD type
  ELSEIFDIF <arg1>, <eax>
    ; Move first value to EAX if it's not EAX
    mov    eax, arg1  
  ENDIF
  cmp    eax, arg2    ; compare arg1 to arg2
  cmovg  eax, arg2    ; move arg2 to EAX if EAX is greater than arg2      
  cmp    eax, arg3    ; compare arg1 to arg3
  cmovg  eax, arg3    ; move arg3 to EAX if EAX is greater than arg3     
ENDM

In the above MACRO the OPATTR (arg1)) AND 00000100b checks if arg1 is a constant value. This is important, because the CMOVcc instructions do not take constants as arguments. You could extend the MACRO to move constants to registers or temporary variables with conditional assembly, but this is not realized in the above code.

The SIZE TYPE arg1 NE 4 directives check if the size of the argument if 4, register or memory - preventing a possible error with the MOV. You could extend this to use MOVZX/MOVSX with further conditional assembly.

The IFDIF <arg1>, <eax> checks if the first argument is equal to the string "EAX". This is done to avoid setting the EAX register if the value is already in it.

Errors occurring are ECHOed to the console starting with a ### providing additional information at assembly time.

This MACRO is far from being complete. It only takes care of some possible errors and could be extended to realize even more automatisation. But IMHO it shows an approach worth of being comtemplated.

zx485
  • 28,498
  • 28
  • 50
  • 59
  • Arg1 *could* be a constant, and get `mov`ed into EAX, without making your macro logic more complex. (Unless that breaks the size check...) Are you sure it's a good idea to put the `mov` check as an `elifdef` after a bunch of error / sanity checks? Would seem to make more sense as something totally separate, so e.g. printing a size warning for arg1 doesn't kill the `mov`. – Peter Cordes Oct 29 '20 at 04:37
  • Unfortunately it _did break_ the SIZE check, so I added a separate branch for it. Because I have no idea how to merge the IFDIF with the IF OPATTR in one IF clause. – zx485 Oct 29 '20 at 05:01
  • `IFDIF , ` will be false for a numeric literal, so you can still use that, as long as it's a separate chain from the warning IF/ELSE chain so it's definitely reached. With your way, `ELSEIF ((OPATTR (arg1)) AND 00000100b)` being true means size never gets checked for arg2 or arg3. So that's not ideal either; again they're separate checks that don't need to be part of an else chain. This doesn't have to be perfect, and making it too huge will end up hiding the actual important part of the answer, but it would be nice to have no false-positive warnings... – Peter Cordes Oct 29 '20 at 05:07