llvm dev - Aug 2015 - [LLVMdev] SelectionDAG viewers, filter-view-dags question

Daniel,

  Ok, thanks. Simple fix. We'll just make correction in local copy for now,
one less thing to port later :)

Thanks.

On Mon, Jul 27, 2015 at 12:29 PM, Daniel Sanders <Daniel.Sanders at
imgtec.com>
wrote:
>  I've just looked at my checkout of 3.6 and it looks like the fix
wasn't
> merged. I don't have the revision number for the fix to hand but in
> lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp, this:
>
>   MatchFilterBB = (!FilterDAGBasicBlockName.empty() &&
>
>                    FilterDAGBasicBlockName =>
>                       
FuncInfo->MBB->getBasicBlock()->getName().str());
>
> Should be:
>
>   MatchFilterBB = (FilterDAGBasicBlockName.empty() ||
>
>                    FilterDAGBasicBlockName =>
>                       
FuncInfo->MBB->getBasicBlock()->getName().str());
>
>
>
> The trunk has the correct code so the option should be ok for LLVM 3.7.
>
>
>
> *From:* Ryan Taylor [mailto:ryta1203 at gmail.com]
> *Sent:* 27 July 2015 17:20
> *To:* Daniel Sanders
> *Cc:* llvmdev at cs.uiuc.edu
> *Subject:* Re: [LLVMdev] SelectionDAG viewers, filter-view-dags question
>
>
>
> Daniel,
>
>
>
>   We are using 3.6. Someone also pointed out that it was mandatory in
> 3.6.2 but I have not verified that.
>
>
>
>
>
> On Mon, Jul 27, 2015 at 12:10 PM, Daniel Sanders <
> Daniel.Sanders at imgtec.com> wrote:
>
> It's not supposed to be. There was a short period where it was
> unintentionally mandatory but this was fixed after I pointed it out during
> post-commit review.
>
>
>
> Which version/revision are you using?
>
>
>
> *From:* llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu] *On
> Behalf Of *Ryan Taylor
> *Sent:* 27 July 2015 16:27
> *To:* llvmdev at cs.uiuc.edu
> *Subject:* [LLVMdev] SelectionDAG viewers, filter-view-dags question
>
>
>
> Is this option currently mandatory? If so, why? If not, I'm not sure
> what's been added that I need to do differently.
>
>
>
> -view-isel-dags opened just fine in dotty in 3.4 and now this does nothing
> without the filter-view-dags 'option' and now has a different
priority
> program list or something.
>
>
>
> I'm just curious why this option should be mandatory?
>
>
>
> Thanks.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150727/64e70d94/attachment.html>

The diff is not only the && and || but also the leading !:


diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 58f029fbe9fc..7ee06fc153b2 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -659,7 +659,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
   (void)BlockNumber;
   bool MatchFilterBB = false; (void)MatchFilterBB;
 #ifndef NDEBUG
-  MatchFilterBB = (!FilterDAGBasicBlockName.empty() &&
+  MatchFilterBB = (FilterDAGBasicBlockName.empty() ||
                    FilterDAGBasicBlockName =                       
FuncInfo->MBB->getBasicBlock()->getName().str());
 #endif


— 
Mehdi


> On Aug 1, 2015, at 11:07 AM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
> 
> Simply replacing the && with || did not fix the issue. This issue
still exists after making those changes. There is maybe some other code that
needs to be changed also?
> 
> Thanks.
> 
> On Mon, Jul 27, 2015 at 12:45 PM, Ryan Taylor <ryta1203 at gmail.com
<mailto:ryta1203 at gmail.com>> wrote:
> Daniel,
> 
>   Ok, thanks. Simple fix. We'll just make correction in local copy for
now, one less thing to port later :)
> 
> Thanks.
> 
> On Mon, Jul 27, 2015 at 12:29 PM, Daniel Sanders <Daniel.Sanders at
imgtec.com <mailto:Daniel.Sanders at imgtec.com>> wrote:
> I've just looked at my checkout of 3.6 and it looks like the fix
wasn't merged. I don't have the revision number for the fix to hand but
in lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp, this:
> 
>   MatchFilterBB = (!FilterDAGBasicBlockName.empty() &&
> 
>                    FilterDAGBasicBlockName => 
>                       
FuncInfo->MBB->getBasicBlock()->getName().str());
> 
> Should be:
> 
>   MatchFilterBB = (FilterDAGBasicBlockName.empty() ||
> 
>                    FilterDAGBasicBlockName => 
>                       
FuncInfo->MBB->getBasicBlock()->getName().str());
> 
>  
> 
> The trunk has the correct code so the option should be ok for LLVM 3.7.
> 
>  
> 
> From: Ryan Taylor [mailto:ryta1203 at gmail.com <mailto:ryta1203 at
gmail.com>]
> Sent: 27 July 2015 17:20
> To: Daniel Sanders
> Cc: llvmdev at cs.uiuc.edu <mailto:llvmdev at cs.uiuc.edu>
> Subject: Re: [LLVMdev] SelectionDAG viewers, filter-view-dags question
> 
>  
> 
> Daniel,
> 
>  
> 
>   We are using 3.6. Someone also pointed out that it was mandatory in 3.6.2
but I have not verified that.
> 
>  
> 
>  
> 
> On Mon, Jul 27, 2015 at 12:10 PM, Daniel Sanders <Daniel.Sanders at
imgtec.com <mailto:Daniel.Sanders at imgtec.com>> wrote:
> 
> It's not supposed to be. There was a short period where it was
unintentionally mandatory but this was fixed after I pointed it out during
post-commit review.
> 
>  
> 
> Which version/revision are you using?
> 
>  
> 
> From: llvmdev-bounces at cs.uiuc.edu <mailto:llvmdev-bounces at
cs.uiuc.edu> [mailto:llvmdev-bounces at cs.uiuc.edu
<mailto:llvmdev-bounces at cs.uiuc.edu>] On Behalf Of Ryan Taylor
> Sent: 27 July 2015 16:27
> To: llvmdev at cs.uiuc.edu <mailto:llvmdev at cs.uiuc.edu>
> Subject: [LLVMdev] SelectionDAG viewers, filter-view-dags question
> 
>  
> 
> Is this option currently mandatory? If so, why? If not, I'm not sure
what's been added that I need to do differently.
> 
>  
> 
> -view-isel-dags opened just fine in dotty in 3.4 and now this does nothing
without the filter-view-dags 'option' and now has a different priority
program list or something.
> 
>  
> 
> I'm just curious why this option should be mandatory?
> 
>  
> 
> Thanks.
> 
>  
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150801/06f1c690/attachment.html>