PLC-lang / rusty

Structured Text Parser and LLVM Frontend
GNU Lesser General Public License v3.0
223 stars 53 forks source link

An error occurred during division of an unsigned variable #1272

Open chen4s666 opened 3 months ago

chen4s666 commented 3 months ago

This is my first time to mention bugs, if there are inappropriate places please guide. Describe the bug An error occurred during division of an unsigned variable. I tested the variable types BYTE, USINT, UINT, and UDINT. They performed division calculations in accordance with signed types. Since both USINT and UINT are extended to 32-bit calculations, this error is avoided.However, I believe that extended computing is also incorrect, which consumes unnecessary resources.

To Reproduce You can see that in the division calculation, sdiv is used for signed integer division

{external}
FUNCTION printf : DINT
VAR_INPUT {ref}
format : STRING;
END_VAR
VAR_INPUT
args: ...;
END_VAR
END_FUNCTION

FUNCTION main:DINT
VAR
    a:BYTE;
    b:BYTE;
    c:BYTE;
    aa:USINT;
    bb:USINT;
    cc:USINT;
    aaa:UINT;
    bbb:UINT;
    ccc:UINT;
    aaaa:UDINT;
    bbbb:UDINT;
    cccc:UDINT;
END_VAR
    b:=64;
    c:=255;
    a:=b/c;
    printf('a:%d',a);
    aa:=bb/cc;
    aaa:=bbb/ccc;
    aaaa:=bbbb/cccc;
END_FUNCTION
@utf08_literal_0 = private unnamed_addr constant [5 x i8] c"a:%d\00"

declare i32 @printf(i8*, ...) section "fn-printf:i32[ps8u81]"

define i32 @main() section "fn-main:i32" {
entry:
  %main = alloca i32, align 4
  %a = alloca i8, align 1
  %b = alloca i8, align 1
  %c = alloca i8, align 1
  %aa = alloca i8, align 1
  %bb = alloca i8, align 1
  %cc = alloca i8, align 1
  %aaa = alloca i16, align 2
  %bbb = alloca i16, align 2
  %ccc = alloca i16, align 2
  %aaaa = alloca i32, align 4
  %bbbb = alloca i32, align 4
  %cccc = alloca i32, align 4
  store i8 0, i8* %a, align 1
  store i8 0, i8* %b, align 1
  store i8 0, i8* %c, align 1
  store i8 0, i8* %aa, align 1
  store i8 0, i8* %bb, align 1
  store i8 0, i8* %cc, align 1
  store i16 0, i16* %aaa, align 2
  store i16 0, i16* %bbb, align 2
  store i16 0, i16* %ccc, align 2
  store i32 0, i32* %aaaa, align 4
  store i32 0, i32* %bbbb, align 4
  store i32 0, i32* %cccc, align 4
  store i32 0, i32* %main, align 4
  store i8 64, i8* %b, align 1
  store i8 -1, i8* %c, align 1

  %load_b = load i8, i8* %b, align 1
  %load_c = load i8, i8* %c, align 1
  %tmpVar = sdiv i8 %load_b, %load_c
  store i8 %tmpVar, i8* %a, align 1
  %load_a = load i8, i8* %a, align 1
  %0 = zext i8 %load_a to i32
  %call = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([5 x i8], [5 x i8]* @utf08_literal_0, i32 0, i32 0), i32 %0)
  %load_bb = load i8, i8* %bb, align 1
  %1 = zext i8 %load_bb to i32
  %load_cc = load i8, i8* %cc, align 1
  %2 = zext i8 %load_cc to i32
  %tmpVar1 = sdiv i32 %1, %2
  %3 = trunc i32 %tmpVar1 to i8
  store i8 %3, i8* %aa, align 1
  %load_bbb = load i16, i16* %bbb, align 2
  %4 = zext i16 %load_bbb to i32
  %load_ccc = load i16, i16* %ccc, align 2
  %5 = zext i16 %load_ccc to i32
  %tmpVar2 = sdiv i32 %4, %5
  %6 = trunc i32 %tmpVar2 to i16
  store i16 %6, i16* %aaa, align 2
  %load_bbbb = load i32, i32* %bbbb, align 4
  %load_cccc = load i32, i32* %cccc, align 4
  %tmpVar3 = sdiv i32 %load_bbbb, %load_cccc
  store i32 %tmpVar3, i32* %aaaa, align 4
  %main_ret = load i32, i32* %main, align 4
  ret i32 %main_ret
}

Expected behavior The correct way to do this is to use udiv for unsigned integer computation.

Tests The calculation in the above print is 192

ghaith commented 3 months ago

The 32 bit conversion is inspired by what C does in such operations. I agree that the division with unsigned values should be udiv.