This is an archive of the discontinued LLVM Phabricator instance.

[X86] Improve select of constants
ClosedPublic

Authored by kazu on Feb 20 2023, 11:14 PM.

Details

Summary

Without this patch:

%cmp = icmp eq i32 %a, %b
%cond = select i1 %cmp, i32 1, i32 2

is compiled as:

31 c9                      xor    %ecx,%ecx
39 f7                      cmp    %esi,%edi
0f 94 c1                   sete   %cl
b8 02 00 00 00             mov    $0x2,%eax
29 c8                      sub    %ecx,%eax

With this patch, the compiler generates:

31 c0                      xor    %eax,%eax
39 f7                      cmp    %esi,%edi
0f 95 c0                   setne  %al
ff c0                      inc    %eax

saving 5 bytes while reducing register usage.

This patch transforms C - setcc into inverted_setcc + (C-1) if C is a
nonzero constant.

This patch fixes:

https://github.com/llvm/llvm-project/issues/60854

Diff Detail

Event Timeline

kazu created this revision.Feb 20 2023, 11:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2023, 11:14 PM
kazu requested review of this revision.Feb 20 2023, 11:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2023, 11:14 PM
ValZapod added a subscriber: ValZapod.EditedFeb 23 2023, 4:38 PM

This is what gcc does only on Os. Not even on -O3. I mean inc eax part. I even found the relevant comment on stackoverflow, https://stackoverflow.com/questions/13383407/is-add-1-really-faster-than-inc-x86#comment103469506_37095553

@kazu Please can you rebase? I've added i686 and slow-inc test coverage to help check for regressions

kazu updated this revision to Diff 501004.Feb 27 2023, 7:43 PM

Rebase.

kazu added a comment.Feb 27 2023, 7:43 PM

@kazu Please can you rebase? I've added i686 and slow-inc test coverage to help check for regressions

Just rebased. Please see the updated .ll tests.

RKSimon added inline comments.Feb 28 2023, 1:07 AM
llvm/test/CodeGen/X86/select_const.ll
1091

it looks like you manually updated these as the script would have removed these check lines - regenerate again?

kazu updated this revision to Diff 501199.Feb 28 2023, 10:03 AM

Regenerate.

kazu marked an inline comment as done.Feb 28 2023, 10:03 AM

Please take a look. Thanks!

This revision is now accepted and ready to land.Feb 28 2023, 10:26 AM
goldstein.w.n added inline comments.Feb 28 2023, 10:38 AM
llvm/test/CodeGen/X86/select-constant-xor.ll
61

Not that it needs to be fixed now, but shouldn't this be:

shr $63, %rdi
lea  2147483647(%rdi),%eax

?

RKSimon added inline comments.Feb 28 2023, 11:59 AM
llvm/test/CodeGen/X86/select-constant-xor.ll
61

@goldstein.w.n Please can you raise a ticket?

goldstein.w.n added inline comments.Feb 28 2023, 12:00 PM
llvm/test/CodeGen/X86/select-constant-xor.ll
61

Working on a patch for it, will have one up soon.

goldstein.w.n added inline comments.Feb 28 2023, 1:21 PM
llvm/test/CodeGen/X86/select-constant-xor.ll
61
This revision was landed with ongoing or failed builds.Feb 28 2023, 3:04 PM
This revision was automatically updated to reflect the committed changes.