[clang++][instruction re-order][performance bug]

[xiaoqiang-wang]

I‘m using clang++ to compile my below C++ code,

static const INSTR_SIZE  =  64;
uint64_t enc;
uint32_t temp;


//void __attribute__((optimize("-fno-reorder-blocks-and-partition")))  set_field(uint8_t pos, uint8_t size, instr_size_t val)

//void __attribute__((optimize("-fno-reorder-blocks")))  set_field(uint8_t pos, uint8_t size, instr_size_t val) 

void  set_field(uint8_t pos, uint8_t size, instr_size_t val) {

  instr_size_t mask = ((1ULL << size) - 1);

  assert(((val & mask) == val) && "Value too wide for field");

  if (pos < INSTR_SIZE) {
    assert(pos + size <= INSTR_SIZE);

    enc &= ~(mask << pos);
    enc |= (val << pos);
  }
  else {
    assert(pos + size <= INSTR_SIZE + 32);

    pos &= (INSTR_SIZE - 1);
    temp &= ~(mask << pos);
    temp |= (val << pos);
  }
}

but there is a performance bug caused by instruction re-order or instruction-combine.

I want the code first try go into the if-block, then go to else-block.
but since of const-folding and instruction-reorder,
the clang++ make code work as below order:

temp &=~(mask<<63)
if(pos < 64){
...
}
else{
...
}

I do not want calculate temp &=~(mask<<63) first.

---

I tried __attribute__((optimize("-fno-reorder-blocks"))) , want to disable instruction-reorder, but it seems not work.

Q1: Is there any way to disable instruction-reorder for this function?

---

below is my machine info

user_name: work_dir $ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.5 LTS
Release:	22.04
Codename:	jammy

user_name: work_dir $ llvm-readelf -p .comment my_project_exe

String dump of section '.comment':
[     0] Linker: LLD 20.0.0 (/mnt/disks/build-disk/src/android/llvm-r547379-release/out/llvm-project/llvm b718bcaf8c198c82f3021447d943401e3ab5bd54)
[    8b] GCC: (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3
[    b5] GCC: (GNU) 4.8.3
[    c7] Android (13290119, +pgo, +bolt, +lto, +mlgo, based on r547379) clang version 20.0.0 (https://android.googlesource.com/toolchain/llvm-project b718bcaf8c198c82f3021447d943401e3ab5bd54)

[Explorer09]

My usual way of suppressing an optimization is to add inline assembly that tricks the compiler into thinking some register values will be clobbered but actually aren't.

An example:

  if (pos < INSTR_SIZE) {
    assert(pos + size <= INSTR_SIZE);

    __asm__("" : "+r" (mask));
    enc &= ~(mask << pos);
    enc |= (val << pos);
  }
  else {
    assert(pos + size <= INSTR_SIZE + 32);

    pos &= (INSTR_SIZE - 1);
    __asm__("" : "+r" (mask));
    temp &= ~(mask << pos);
    temp |= (val << pos);
  }

Note that I don't consider what you reported is actually a bug.

[xiaoqiang-wang]

@dtcxzyw @Explorer09 @frederick-vs-ja

Note that I don't consider what you reported is actually a bug.

Sir, Thanks for you replay very much.

My APP will call the set_field() API many times.
In most situation, it will go into the if-block, not go into the else-block.

For if-block, the pre-calculate of temp &=~(mask<<63) is redundant,
and it lead to performance decline.

I think this is a performance bug,

---

Q1: Will LLVM-team consider to fix this bug?

[Explorer09]

For if-block, the pre-calculate of temp &=~(mask<<63) is redundant, and it lead to performance decline.

I think this is a performance bug,

Without the machine code output, it's difficult to tell where your performance problem is.

For your particular example, since the expression ~(mask << pos) is the same for both branches, it is normal for the compiler to take out and precompute that expression. The same could happen on (val << pos), too.

[xiaoqiang-wang]

@Explorer09

since the expression ~(mask << pos) is the same for both branches

bro, Thanks for your replay very much.

In the if-block and else-block , the ~(mask << pos) statement just looks same ,
But it is completely different statement for if-block and else-block.

---

1. in if-block :
   a. the pos is a variable.
   pos is the the function's input argument.
   b. compiler must first go-into the if-block, then calculate the ~(mask << pos) operation.
   c. compiler can not pre-calculation ~(mask << pos) before the if-block.

2. in else-block:
   a. the pos is a constant.
   pos was write by pos &= (INSTR_SIZE - 1), it's value is 63.
   b. so, compiler can pre-calculate the ~(mask << pos) before the if-else statement.
   c. but this will lead to redundant calculation, when we go-into if-block.

I'm trying to get the demo code from my project.

[Explorer09]

1. in if-block :
   a. the pos is a variable.
   pos is the the function's input argument.
   b. compiler must first go-into the if-block, then calculate the ~(mask << pos) operation.
   c. compiler can not pre-calculation ~(mask << pos) before the if-block.

Consider that the left shift behavior would be undefined if pos is >= 64. I can't find a reason not to pre-calculate ~(mask << pos) in your case.

2. in else-block:
   a. the pos is a constant.
   pos was write by pos &= (INSTR_SIZE - 1), it's value is 63.
   b. so, compiler can pre-calculate the ~(mask << pos) before the if-else statement.
   c. but this will lead to redundant calculation, when we go-into if-block.

Note that in x86, the left shift instruction has a mod-64 behavior by default. What I mean is that pos &= (INSTR_SIZE - 1) could become a no-op if the pos is used only for the left shift operation.

Again, I'm guessing your compile target (x86) since you didn't specify that detail yet.