<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Yeah, that was my understanding and it was I meant to do.<div class="">The fact is that it was mentioned that we shouldn’t use PseudoSourceValue, because it was planned to be removed .</div><div class="">But probably that is not the case apparently?</div><div class=""><br class=""></div><div class="">Thanks for the comment on the patch. I’ll take a look at the link you posted and do what is needed :)</div><div class=""><br class=""></div><div class="">Marcello</div><div class=""><div><blockquote type="cite" class=""><div class="">On 22 Sep 2015, at 16:44, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">----- Original Message -----</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">From: "Marcello Maggioni via llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>><br class="">To: "Nick Lewycky" <<a href="mailto:nicholas@mxc.ca" class="">nicholas@mxc.ca</a>><br class="">Cc: <a href="mailto:reviews+D12920+public+8f10800ecfd8d899@reviews.llvm.org" class="">reviews+D12920+public+8f10800ecfd8d899@reviews.llvm.org</a>, "LLVM Commits" <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>><br class="">Sent: Friday, September 18, 2015 1:17:15 AM<br class="">Subject: Re: [PATCH] D12920: Add TargetCustom type to PseudoSourceValue<br class=""><br class="">HI Nick, thanks for the comment.<br class=""><br class="">When you mention "use MachinePointerInfo" what do you mean exactly?<br class=""><br class="">Creating one leaving the Value to null.<br class="">The approach I was using was creating a MachinePointerInfo and<br class="">attaching target specific data to a PSV and pass the PSV to the<br class="">constructor of the MachinePointerInfo. The idea was to use that<br class="">target specific data later on in target passes or in functions like<br class="">"isTriviallyDisjoint".<br class="">MachinePointerInfo alone doesn't seem to have any way of storing such<br class="">data with the exclusion of the offset field. Are you suggesting of<br class="">using that to deliver target specific information?<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I don't think that sticking non-offset data into the Offset field is a good idea. Target-independent code might legitimately interpret that field as specifying a relative offset even if it does not understand the underlying PseudoSourceValue.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I would expect that you would derive a subclass from PseudoSourceValue and give it whatever fields were necessary to store your target-specific data, just as we currently do with the target-independent subclasses of PseudoSourceValue (FixedStackPseudoSourceValue, etc.)</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Also, please re-post your patch with full context (see<span class="Apple-converted-space"> </span></span><a href="http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">).</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">-Hal</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">Thanks,<br class="">Marcello<br class=""><br class="">Sent from my iPhone<br class=""><br class=""><blockquote type="cite" class="">On Sep 17, 2015, at 11:00 PM, Nick Lewycky via llvm-commits<br class=""><<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Pete Cooper wrote:<br class=""><blockquote type="cite" class="">I might be mis-remembering, but I thought we were trying to remove<br class="">PSV’s?<br class=""><br class="">Only commit I could see which remotely suggested that might be the<br class="">case is r206255 where the comment contained<br class=""><br class="">"Anything that needs to use either a PseudoSourceValue* and Value*<br class="">is strongly encouraged to use a MachinePointerInfo instead.”<br class=""><br class="">I’ve CCed Nick to see if he can remember whether there was more<br class="">work to come on PSV’s or whether they are here to stay. If they<br class="">are going to be removed in future then its something to think<br class="">about here before adding more code which relies on them.<br class=""></blockquote><br class="">The really important part happened, PSV's no longer derive from<br class="">Value*. The advice about using MachinePointerInfo when possible<br class="">remains true, but my work on removing PSV entirely is unlikely to<br class="">be completed.<br class=""><br class="">Nick<br class=""><br class=""><blockquote type="cite" class=""><br class="">Pete<br class=""><blockquote type="cite" class="">On Sep 17, 2015, at 5:51 PM, Marcello Maggioni via<br class="">llvm-commits<<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">kariddi updated this revision to Diff 35051.<br class="">kariddi marked an inline comment as done.<br class="">kariddi added a comment.<br class=""><br class="">Still waiting for comments about the MIR serialization stuff.<br class=""><br class=""><br class=""><a href="http://reviews.llvm.org/D12920" class="">http://reviews.llvm.org/D12920</a><br class=""><br class="">Files:<br class="">include/llvm/CodeGen/PseudoSourceValue.h<br class="">lib/CodeGen/MIRPrinter.cpp<br class=""><br class="">Index: lib/CodeGen/MIRPrinter.cpp<br class="">===================================================================<br class="">--- lib/CodeGen/MIRPrinter.cpp<br class="">+++ lib/CodeGen/MIRPrinter.cpp<br class="">@@ -892,6 +892,9 @@<br class=""> printLLVMNameWithoutPrefix(<br class=""> OS,<br class=""> cast<ExternalSymbolPseudoSourceValue>(PVal)->getSymbol());<br class=""> break;<br class="">+ case PseudoSourceValue::TargetCustom:<br class="">+ llvm_unreachable("TargetCustom pseudo source values are<br class="">not supported");<br class="">+ break;<br class=""> }<br class=""> }<br class=""> printOffset(Op.getOffset());<br class="">Index: include/llvm/CodeGen/PseudoSourceValue.h<br class="">===================================================================<br class="">--- include/llvm/CodeGen/PseudoSourceValue.h<br class="">+++ include/llvm/CodeGen/PseudoSourceValue.h<br class="">@@ -40,7 +40,8 @@<br class=""> ConstantPool,<br class=""> FixedStack,<br class=""> GlobalValueCallEntry,<br class="">- ExternalSymbolCallEntry<br class="">+ ExternalSymbolCallEntry,<br class="">+ TargetCustom<br class=""> };<br class=""><br class="">private:<br class="">@@ -63,6 +64,9 @@<br class=""> bool isGOT() const { return Kind == GOT; }<br class=""> bool isConstantPool() const { return Kind == ConstantPool; }<br class=""> bool isJumpTable() const { return Kind == JumpTable; }<br class="">+ unsigned getTargetCustom() const {<br class="">+ return (Kind>= TargetCustom) ? ((Kind+1) - TargetCustom) :<br class="">0;<br class="">+ }<br class=""><br class=""> /// Test whether the memory pointed to by this<br class=""> PseudoSourceValue has a<br class=""> /// constant value.<br class=""><br class=""><br class=""><D12920.35051.patch>_______________________________________________<br class="">llvm-commits mailing list<br class="">llvm-commits@lists.llvm.org<br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></blockquote></blockquote><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></blockquote>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hal Finkel</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Assistant Computational Scientist</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Leadership Computing Facility</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Argonne National Laboratory</span></div></blockquote></div><br class=""></div></body></html>