[PATCH] D37289: [X86] Speculatively load operands of select instruction

David Kreitzer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 6 14:06:50 PDT 2017


DavidKreitzer added a comment.

After some internal discussions, we suspect that Sanjay's counter-example may have undefined behavior according to the C standard.

  typedef struct S {
    char padding[4088];
    struct S *p1;
    struct S *p2;
  } S;
  
  S* f1(S *s, int x)
  {
    S *r;
    if (x)
      r = s->p1;
    else
      r = s->p2;
    return r;
  }

Sanjay's example was to pass the address of an incomplete object of type S as the first argument to f1. f1 will always access that object through an lvalue of type S, which seems like a violation of the type based aliasing rules in section 6.5 paragraph 7 of the standard:

> An object shall have its stored value accessed only by an lvalue expression that has one of
>  the following types:
>  — a type compatible with the effective type of the object,

I am deliberately using fuzzy wording like "may have" and "seems like". I would be happy to have the C language experts in the community weigh in on whether this is valid optimization. But both gcc and icc speculate the loads in f1. This is the code generated by gcc 7.2 at -O2:

  f1:
          movq    4096(%rdi), %rax
          testl   %esi, %esi
          cmovne  4088(%rdi), %rax
          ret

icc is admittedly more aggressive with this optimization. gcc will only speculate the loads when they are accessing adjacent fields of the same struct. But both compilers are using the same logic that accessing one field of a structure makes it safe to speculatively access another field of the same structure.

Note that I am not trying to argue that this patch is correct. It isn't, because it doesn't guard against the case where the same base pointer is used to access structs of two different types, e.g.

  typedef struct S {
    char padding[4088];
    struct S *p1;
    struct S *p2;
  } S;
  
  // T is a partial overlay of S
  typedef struct T {
    char padding[4088];
    struct S *p1;
  } T;
  
  S* f1(S *s, int x)
  {
    S *r;
    if (x)
      // p1 is accessed through an lvalue of type T. So it isn't safe to speculate the load of p2.
      r = ((T*)s)->p1;
    else
      r = s->p2;
    return r;
  }

I'd like to pose two questions.

1. Is the gcc & icc logic correct? It is valid to speculatively access a structure field in the presence of an access to a different field of the same structure?
2. If yes, how can we perform this optimization in LLVM? Should the TBAA information be used for this purpose, e.g. to enhance the isSafeToLoadUnconditionally utility that Hal mentioned?


https://reviews.llvm.org/D37289





More information about the llvm-commits mailing list