[PATCH] [globalsmodref-aa] Atomics *DO* touch memory...

[Hal Finkel]

----- Original Message -----
> From: "Sean Silva" <chisophugis at gmail.com>
> To: llvm-commits at cs.uiuc.edu
> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Nick Lewycky" <nlewycky at google.com>, "Philip Reames" <listmail at philipreames.com>
> Sent: Friday, April 24, 2015 10:56:58 PM
> Subject: [PATCH] [globalsmodref-aa] Atomics *DO* touch memory...
> 
> 
> GlobalsModRef was giving a spurious alias analysis result because it
> failed to consider that atomics access memory. The attached patch
> fixes that (PR23345).
> 
> 
> I reduced this from a "compare and swap" loop that looked funny in
> the machine code: the "compare and swap" had in fact been hoisted
> out of the loop. I only ran into this when I tried `-flto -O2
> -fno-inline`.
> 
> 
> The structure of the test is pretty closely copied from one of the
> others in the directory. Any better ideas for testing this (or ways
> to make the testing more thorough) would be appreciated.
> 
> 
> If somebody can doublecheck that these are the only missing cases,
> that would be appreciated. This loop been touched since 2012 and I'm
> relatively clueless about AA. Hopefully this patch also suggests a
> couple other places to audit for similar bugs to those with a better
> "big picture" of the IR analysis and transforms than myself.

Hi Sean,

The list is also missing VAArgInst, although maybe that's on purpose (although, in that case, a comment would be nice).

While we're looking at this, other things I don't understand about this loop are 1) Why is is special casing alloc/free functions and 2) why is it treating all volatile accesses as read/write?

It might be nice to refactor this code to call
  ModRefResult getModRefInfo(const Instruction *I)
from AliasAnalysis. That way we could reduce the number of places that replicate this kind of logic.

 -Hal

> 
> 
> -- Sean Silva

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory