On 16 Nov 2016, at 19:03, Davide Italiano <davide at FreeBSD.org> wrote:> >>> >>> For our target, this is only sound if you can show that the pointer was >>> used to read all of the bytes that you are loading (we have byte-granularity >>> memory safety). Old GVN has no hooks for targets to specify whether this is >>> safe and so is implicitly assuming a page-based MMU. This optimisation is >>> also unsound for M-profile ARM cores, though will fail occasionally there, >>> whereas it fails deterministically for us. >>> >>> Does it make any assumptions about the layout of memory in pointers? >> > > Which assumptions are you thinking of?My last merge from upstream was about a year ago (and a new one is long overdue), but there were issues where GVN was assuming that if it did a load of a pointer then a ptrtoint, then a truncation, that it would get the same result as doing a narrower load. This is not the case in any platform where pointers are not simply integers (i.e. where you actually need inttoptr / ptrtoint instead of bitcast). David
On Wed, Nov 16, 2016 at 11:11 AM, David Chisnall < David.Chisnall at cl.cam.ac.uk> wrote:> On 16 Nov 2016, at 19:03, Davide Italiano <davide at FreeBSD.org> wrote: > > > >>> > >>> For our target, this is only sound if you can show that the pointer was > >>> used to read all of the bytes that you are loading (we have > byte-granularity > >>> memory safety). Old GVN has no hooks for targets to specify whether > this is > >>> safe and so is implicitly assuming a page-based MMU. This > optimisation is > >>> also unsound for M-profile ARM cores, though will fail occasionally > there, > >>> whereas it fails deterministically for us. > >>> > >>> Does it make any assumptions about the layout of memory in pointers? > >> > > > > Which assumptions are you thinking of? > > My last merge from upstream was about a year ago (and a new one is long > overdue), but there were issues where GVN was assuming that if it did a > load of a pointer then a ptrtoint, then a truncation, that it would get the > same result as doing a narrower load. This is not the case in any platform > where pointers are not simply integers (i.e. where you actually need > inttoptr / ptrtoint instead of bitcast). >You keep talking about platforms, but llvm ir itself is not platform dependent. Can you give a reference in the language reference that says that this is not legal? IE what loads do *on your platform* is completely irrelevant to whether the IR code is legal or not, only what it codegens to. LLVM's type semantics (and pointers may not have types, but the load operations produce values that do) are also not defined in terms of platform, but in terms of what datalayout says, etc. What you want seems to be non-integral pointer types. Which are experimental: "LLVM IR optionally allows the frontend to denote pointers in certain address spaces as “non-integral” via the datalayout string <http://llvm.org/docs/LangRef.html#langref-datalayout>. Non-integral pointer types represent pointers that have an *unspecified* bitwise representation; that is, the integral representation may be target dependent or unstable (not backed by a fixed integer). inttoptr instructions converting integers to non-integral pointer types are ill-typed, and so are ptrtoint instructions converting values of non-integral pointer types to integers. Vector versions of said instructions are ill-typed as well." One of the reasons it's experimental is because nobody has made it work in all cases. I think whoever wants this to work is going to have to drive fixing it and making it work sanely.> David > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161116/03851d17/attachment-0001.html>
Hi all, Daniel Berlin via llvm-dev wrote:> My last merge from upstream was about a year ago (and a new one is > long overdue), but there were issues where GVN was assuming that if > it did a load of a pointer then a ptrtoint, then a truncation, that > it would get the same result as doing a narrower load. This is not > the case in any platform where pointers are not simply integers > (i.e. where you actually need inttoptr / ptrtoint instead of bitcast). > > > You keep talking about platforms, but llvm ir itself is not platform > dependent. > Can you give a reference in the language reference that says that this > is not legal? > > IE what loads do *on your platform* is completely irrelevant to whether > the IR code is legal or not, only what it codegens to. > > LLVM's type semantics (and pointers may not have types, but the load > operations produce values that do) are also not defined in terms of > platform, but in terms of what datalayout says, etc. > > What you want seems to be non-integral pointer types. > > Which are experimental: > > "LLVM IR optionally allows the frontend to denote pointers in certain > address spaces as “non-integral” via the datalayout string > <http://llvm.org/docs/LangRef.html#langref-datalayout>. Non-integral > pointer types represent pointers that have an /unspecified/ bitwise > representation; that is, the integral representation may be target > dependent or unstable (not backed by a fixed integer). > > |inttoptr| instructions converting integers to non-integral pointer > types are ill-typed, and so are |ptrtoint| instructions converting > values of non-integral pointer types to integers. Vector versions of > said instructions are ill-typed as well." > > > One of the reasons it's experimental is because nobody has made it work > in all cases. > > I think whoever wants this to work is going to have to drive fixing it > and making it work sanely.Hopefully this won't derail this thread -- but I plan to resume work on non-integral pointers very soon (mid December - early Jan). Right now I'm busy with some higher priority things. We have the same problem as David C., btw, that GVN tends to freely convert between pointers and integers. We have local patches that fix old GVN to DTRT, and the my plan was to upstream the custom patches predicated on the pointer types. Same with instcombine (I don't remember if we have other patches). I'm fine re-doing the same work on NewGVN (prevent inttoptr / ptrtoint on certain class of pointers). -- Sanjoy
On 16 Nov 2016, at 21:56, Daniel Berlin <dberlin at dberlin.org> wrote:> > You keep talking about platforms, but llvm ir itself is not platform dependent. > Can you give a reference in the language reference that says that this is not legal?Nothing in the LangRef (apart from the note about non-integral pointers, which was added recently) makes any claim about the representation of pointers. Pointers in LLVM IR have always been opaque and must explicitly be bitcast or inttoptr / ptrtoint cast to be used as if they were integers. We have had discussions on the list previously about tightening up the semantics of inttoptr and ptrtoint.> IE what loads do *on your platform* is completely irrelevant to whether the IR code is legal or not, only what it codegens to. > > LLVM's type semantics (and pointers may not have types, but the load operations produce values that do) are also not defined in terms of platform, but in terms of what datalayout says, etc.GVN is materialising loads that go beyond the bounds of an object. This is undefined behaviour in C and there is nothing in the LangRef that indicates that this should be valid. It is only potentially valid because, on platforms with a page-based MMU as the sole form of memory protection, if you only round up to a power of two then you will still be in the same page (and, likely, cache line) so you will get some unspecified data and can ignore it.> What you want seems to be non-integral pointer types. > > Which are experimental: > "LLVM IR optionally allows the frontend to denote pointers in certain address spaces as “non-integral” via the datalayout string. Non-integral pointer types represent pointers that have an unspecified bitwise representation; that is, the integral representation may be target dependent or unstable (not backed by a fixed integer). > inttoptr instructions converting integers to non-integral pointer types are ill-typed, and so are ptrtoint instructions converting values of non-integral pointer types to integers. Vector versions of said instructions are ill-typed as well." > > One of the reasons it's experimental is because nobody has made it work in all cases. > I think whoever wants this to work is going to have to drive fixing it and making it work sanely.Actually, that isn’t what I want, because we do define inttoptr and ptrtoint for our architecture. You can’t implement C without them (or some equivalent) working and we have a fully working C / Objective-C compiler (C++ in progress) using LLVM. ptrtoint is always valid for us, inttoptr may give null depending on the ABI and environment. I gave a talk in the LLVM track at FOSDEM a couple of years ago about the things that are needed to make LLVM work correctly for targets where integers are not pointers. We have done most of this work, but it is not helped by people propagating the ‘integers are pointers’ assumption (which the LangRef has always been *very* careful not to state) in passes. David