Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoiding "warning: extra assignment introduced:" #175

Closed
tfaoliveira opened this issue Jun 21, 2022 · 2 comments · Fixed by #177
Closed

Avoiding "warning: extra assignment introduced:" #175

tfaoliveira opened this issue Jun 21, 2022 · 2 comments · Fixed by #177

Comments

@tfaoliveira
Copy link
Member

While trying to remove all compilation warnings from libjbn, I reached a point where I was not able to progress. I start by explaining the impact of the issue and then the issue.

In libjbn, for instance, in sike751 instantiation, there are ~130 MOV instructions that are being inserted by the compiler which can (most likely) be removed. These have the form REG1 :r= #MOV_64(REG2); where REG2 is not RSP or RIP.
This also happens in libjade: for instance, in Kyber768 avx2 version I can find ~40 possible occurrences of these and a couple of them in Keccak.

I isolated this behavior in a small example which I attach and inline below. The following Jasmin code:

param int N = 2;

inline fn init() -> stack u64[N]
{
  inline int i;
  stack u64[N] r;
  for i=0 to N
  { r[i] = i; }
  return r;
}

inline fn add(reg ptr u64[N] a b) -> reg ptr u64[N]
{
  reg u64 i t;
  i = 0;
  while(i < N)
  { t = a[(int)i];
    t += b[(int)i];
    a[(int) i] = t;
    i += 1;
  }
  return a;
}

inline fn store(reg u64 p, reg ptr u64[N] x)
{
  reg u64 i t;
  i = 0;
  while(i < N)
  { t = x[(int)i];
    (u64)[p + 8*i] = t;
    i += 1;
  }
}

export fn test1(reg u64 p)
{
  inline int i;
  stack u64[N] a b;
  reg ptr u64[N] ap bp;

  a = init(); ap = a;
  b = init(); bp = b;

  ap = add(ap, bp);
  store(p, ap);
  store(p, bp); 
}

Which produces the following warning:

 "test1.jazz", line 45 (2-19):
warning: extra assignment introduced:
RDX :r= #MOV_64(RCX);

Before the code corresponding to the add function is inlined (ap = add(ap, bp);), a copy of bp is created.
The produced assembly is the following:

_test1:
test1:
  movq  %rsp, %r10
  leaq  -32(%rsp), %rsp
  andq  $-8, %rsp

  movq  $0, (%rsp)           # a = init(); 
  movq  $1, 8(%rsp)
  movq  %rsp, %rax           # ap = a; # ap is rax

  movq  $0, 16(%rsp)         # b = init();
  movq  $1, 24(%rsp)
  leaq  16(%rsp), %rcx       # bp = b; # bp is rcx

  movq  %rcx, %rdx           # "test1.jazz", line 45 (2-19):
                             # warning: extra assignment introduced:
                             # RDX :r= #MOV_64(RCX);

  movq  $0, %rsi             # ap = add(ap, bp);
  jmp   Ltest1$5
Ltest1$6:
  movq  (%rax,%rsi,8), %r8
  addq  (%rdx,%rsi,8), %r8
  movq  %r8, (%rax,%rsi,8)
  incq  %rsi
Ltest1$5:
  cmpq  $2, %rsi
  jb    Ltest1$6


  movq  $0, %rdx             # store(p, ap);
  jmp   Ltest1$3
Ltest1$4:
  movq  (%rax,%rdx,8), %rsi  #   t = x[(int)i]; # where x is ap
  movq  %rsi, (%rdi,%rdx,8)
  incq  %rdx
Ltest1$3:
  cmpq  $2, %rdx
  jb    Ltest1$4


  movq  $0, %rax             # store(p, bp);
  jmp   Ltest1$1
Ltest1$2:
  movq  (%rcx,%rax,8), %rdx  #    t = x[(int)i]; # where rcx is bp
  movq  %rdx, (%rdi,%rax,8)
  incq  %rax
Ltest1$1:
  cmpq  $2, %rax
  jb    Ltest1$2
  movq  %r10, %rsp
  ret  

How can I (or if it is currently possible) avoid these extra assignments (given that they are unecessary and affect performance/code size)?

test1_jazz.txt
test1_s.txt

@tfaoliveira
Copy link
Member Author

I forgot to mention: avoiding these warnings without changing the code to ap, bp = add(ap, bp); (this would be very intrusive);
My current impression is that there should be a compiler flag to prevent the insertion of these extra assignments.

@vbgl
Copy link
Member

vbgl commented Jun 22, 2022

This is a bug in the compiler. I see two possible fixes.

  1. infer the right mutability of variable bp (it should be const but inferred as mut);
  2. or enable the removal of renamings that change the pointer mutability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants