Doerfert, Johannes via llvm-dev
2019-Aug-22 17:32 UTC
[llvm-dev] [RFC] Increase the scope of Value::getPointerXXX methods
Right now, we have Value::getPointerDereferenceableBytes() and Value::getPointerAlignment() that allow to query these properties for a value. However, neither does any "reasoning" that would require looking at other values, e.g., they do pure local lookups. To do actual reasoning we have the "isDereferenceableAndAlignedPointer()"-family of functions in Loads.h. Additionally, various places duplicate parts of the necessary logic to deal with alignment and dereferenceability because the result of the "isDereferenceableAndAlignedPointer" is often not sufficient, e.g., we want the number and not test a specific number. Now I see two ways that would improve the situation here, I mean to actually allow outside user to get dereferenceable bytes and alignment information without copying the logic, we could: (1) Make the Value methods "smarter", or (2) expose more information through helper functions, e.g., a "smart" getDereferenceableBytes and getAlignment that live outside of the Value class. Option (1): Benefit: - Less confusion which methods to use, especially if we add helper function to "compute" the information, e.g., getDereferenceableBytes outside of Value. We would have one set to expose raw information, the Value::getXXXX methods, and one set to have them put in context, the isYYYY helpers. Drawback: - The Value::getXXX methods are not pure lookups for that particular value anymore but they also include logic. Option (2): Benefits and drawbacks are basically the opposite of Option (1). I want to clear this because I have a patch to improve the GEP handling wrt. dereferenceable bytes and alignment. I need improvement in parts because isDereferenceableAndAlignedPointer is "smart" when it comes to dereferenceability of GEPs but not when it is about alignment. Also, the handling we have is not optimal and probably replicated a few times. One last caveat: I'd also like to reuse whatever we come up with in the Attributor framework to not duplicate the logic there. That means I want a way to add "optimistic/assumed" information into a lookup. I guess the cleanest is an optional lookup function that can be provided. We can add it to both Option (1) and (2) but I wanted to mention this already. Cheers, Johannes -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190822/b94914a0/attachment.sig>
Doerfert, Johannes via llvm-dev
2019-Aug-22 21:10 UTC
[llvm-dev] [RFC] Increase the scope of Value::getPointerXXX methods
I sketched a third alternative here: https://reviews.llvm.org/D66618 It is a hybrid between the two other options, it does expose a more powerful version of getPointerXXX but replaces the Value member to avoid confusion. Please let me know which way is preferred. ________________________________________ From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Doerfert, Johannes via llvm-dev <llvm-dev at lists.llvm.org> Sent: Thursday, August 22, 2019 12:32 To: LLVM-Dev Subject: [llvm-dev] [RFC] Increase the scope of Value::getPointerXXX methods Right now, we have Value::getPointerDereferenceableBytes() and Value::getPointerAlignment() that allow to query these properties for a value. However, neither does any "reasoning" that would require looking at other values, e.g., they do pure local lookups. To do actual reasoning we have the "isDereferenceableAndAlignedPointer()"-family of functions in Loads.h. Additionally, various places duplicate parts of the necessary logic to deal with alignment and dereferenceability because the result of the "isDereferenceableAndAlignedPointer" is often not sufficient, e.g., we want the number and not test a specific number. Now I see two ways that would improve the situation here, I mean to actually allow outside user to get dereferenceable bytes and alignment information without copying the logic, we could: (1) Make the Value methods "smarter", or (2) expose more information through helper functions, e.g., a "smart" getDereferenceableBytes and getAlignment that live outside of the Value class. Option (1): Benefit: - Less confusion which methods to use, especially if we add helper function to "compute" the information, e.g., getDereferenceableBytes outside of Value. We would have one set to expose raw information, the Value::getXXXX methods, and one set to have them put in context, the isYYYY helpers. Drawback: - The Value::getXXX methods are not pure lookups for that particular value anymore but they also include logic. Option (2): Benefits and drawbacks are basically the opposite of Option (1). I want to clear this because I have a patch to improve the GEP handling wrt. dereferenceable bytes and alignment. I need improvement in parts because isDereferenceableAndAlignedPointer is "smart" when it comes to dereferenceability of GEPs but not when it is about alignment. Also, the handling we have is not optimal and probably replicated a few times. One last caveat: I'd also like to reuse whatever we come up with in the Attributor framework to not duplicate the logic there. That means I want a way to add "optimistic/assumed" information into a lookup. I guess the cleanest is an optional lookup function that can be provided. We can add it to both Option (1) and (2) but I wanted to mention this already. Cheers, Johannes
Finkel, Hal J. via llvm-dev
2019-Aug-23 05:58 UTC
[llvm-dev] [RFC] Increase the scope of Value::getPointerXXX methods
On 8/22/19 12:32 PM, Doerfert, Johannes via llvm-dev wrote: Right now, we have Value::getPointerDereferenceableBytes() and Value::getPointerAlignment() that allow to query these properties for a value. However, neither does any "reasoning" that would require looking at other values, e.g., they do pure local lookups. To do actual reasoning we have the "isDereferenceableAndAlignedPointer()"-family of functions in Loads.h. Additionally, various places duplicate parts of the necessary logic to deal with alignment and dereferenceability because the result of the "isDereferenceableAndAlignedPointer" is often not sufficient, e.g., we want the number and not test a specific number. Now I see two ways that would improve the situation here, I mean to actually allow outside user to get dereferenceable bytes and alignment information without copying the logic, we could: (1) Make the Value methods "smarter", or (2) expose more information through helper functions, e.g., a "smart" getDereferenceableBytes and getAlignment that live outside of the Value class. We seem to have developed a trend toward putting more of these kinds of stateless analysis functions into the core IR library instead of having them in Analysis. I'm not sure that this is desirable. My inclination is to say that we should have the functions in the Analysis library - and then make them as smart as we'd like without worrying about potential layering restrictions. -Hal Option (1): Benefit: - Less confusion which methods to use, especially if we add helper function to "compute" the information, e.g., getDereferenceableBytes outside of Value. We would have one set to expose raw information, the Value::getXXXX methods, and one set to have them put in context, the isYYYY helpers. Drawback: - The Value::getXXX methods are not pure lookups for that particular value anymore but they also include logic. Option (2): Benefits and drawbacks are basically the opposite of Option (1). I want to clear this because I have a patch to improve the GEP handling wrt. dereferenceable bytes and alignment. I need improvement in parts because isDereferenceableAndAlignedPointer is "smart" when it comes to dereferenceability of GEPs but not when it is about alignment. Also, the handling we have is not optimal and probably replicated a few times. One last caveat: I'd also like to reuse whatever we come up with in the Attributor framework to not duplicate the logic there. That means I want a way to add "optimistic/assumed" information into a lookup. I guess the cleanest is an optional lookup function that can be provided. We can add it to both Option (1) and (2) but I wanted to mention this already. Cheers, Johannes _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190823/084f44a0/attachment.html>
Doerfert, Johannes via llvm-dev
2019-Aug-23 06:18 UTC
[llvm-dev] [RFC] Increase the scope of Value::getPointerXXX methods
On 08/23, Finkel, Hal J. wrote:> > On 8/22/19 12:32 PM, Doerfert, Johannes via llvm-dev wrote: > > Right now, we have Value::getPointerDereferenceableBytes() and > Value::getPointerAlignment() that allow to query these properties for a > value. However, neither does any "reasoning" that would require looking > at other values, e.g., they do pure local lookups. To do actual > reasoning we have the "isDereferenceableAndAlignedPointer()"-family of > functions in Loads.h. Additionally, various places duplicate parts of > the necessary logic to deal with alignment and dereferenceability > because the result of the "isDereferenceableAndAlignedPointer" is often > not sufficient, e.g., we want the number and not test a specific number. > > Now I see two ways that would improve the situation here, I mean to > actually allow outside user to get dereferenceable bytes and alignment > information without copying the logic, we could: > (1) Make the Value methods "smarter", or > (2) expose more information through helper functions, e.g., a "smart" > getDereferenceableBytes and getAlignment that live outside of the > Value class. > > > We seem to have developed a trend toward putting more of these kinds of > stateless analysis functions into the core IR library instead of having them in > Analysis. I'm not sure that this is desirable. My inclination is to say that we > should have the functions in the Analysis library - and then make them as smart > as we'd like without worrying about potential layering restrictions. > > -Halhttps://reviews.llvm.org/D66618 shows how a solution with a single function (per property) in the analysis library could look like. It is not 100% clean and I can first move the code before I add the GEP functionality but the general idea should become clear. The impact on the test, assuming I didn't overlook an error, seems also be to consistently positive (with the execption of the one loop fusion test).> Option (1): > Benefit: > - Less confusion which methods to use, especially if we add helper > function to "compute" the information, e.g., getDereferenceableBytes > outside of Value. We would have one set to expose raw information, > the Value::getXXXX methods, and one set to have them put in context, > the isYYYY helpers. > Drawback: > - The Value::getXXX methods are not pure lookups for that particular > value anymore but they also include logic. > > Option (2): > Benefits and drawbacks are basically the opposite of Option (1). > > > I want to clear this because I have a patch to improve the GEP handling > wrt. dereferenceable bytes and alignment. I need improvement in parts > because isDereferenceableAndAlignedPointer is "smart" when it comes to > dereferenceability of GEPs but not when it is about alignment. Also, the > handling we have is not optimal and probably replicated a few times. > > One last caveat: I'd also like to reuse whatever we come up with in the > Attributor framework to not duplicate the logic there. That means I want > a way to add "optimistic/assumed" information into a lookup. I guess the > cleanest is an optional lookup function that can be provided. We can add > it to both Option (1) and (2) but I wanted to mention this already. > > Cheers, > Johannes > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory >-- Johannes Doerfert Researcher Argonne National Laboratory Lemont, IL 60439, USA jdoerfert at anl.gov -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190823/5470ce7c/attachment.sig>