[GraphWriter] Don't wait on xdg-open when not on Apple

[• chatur01]

Summary

Hi,

I could be way off with this patch, but it seems like the waiting behaviour introduced in 255cd51fbd77 (2012) and its use extended by D6534 won’t work on systems using xdg-open, since there’s no option to have xdg-open wait on the application it launches AFAICT, and if there is, then GraphWriter is not making use of it.

What happens on my computer is that xdg-open launches a dot file viewer, then immediately returns, and then the GraphWriter quickly deletes all the temporary dot files, and then the dot viewers get upset, if they even got a chance to launch.

This patch moves the “wait” behaviour to only Apple platforms, where the “open” command actually has a wait option.

[MatzeB]

The patch itself LGTM.

As we are on the topic: I think the current approach of having llvm spawn the viewer and wait/not wait for it should be changed: I'd really prefer llvm just writing a bunch of .dot files to the current directory (or /tmp) giving them some sensible names+numbers and maybe print those names.

This would avoid llvm having to find and launch the correct viewer. In my experience I also often have cases where n graphs are generated but I am only interested in one of them which forces me to close n-1 graph viewers.

[• chatur01]

As we are on the topic: I think the current approach of having llvm
spawn the viewer and wait/not wait for it should be changed: I'd
really prefer llvm just writing a bunch of .dot files to the current
directory (or /tmp) giving them some sensible names+numbers and maybe
print those names.

This would avoid llvm having to find and launch the correct viewer. In
my experience I also often have cases where n graphs are generated but
I am only interested in one of them which forces me to close n-1 graph
viewers.

I suppose you mean the same thing as the -dot-* options to the opt
tool? I agree that would be handy extension to llc for the
visualisation tools too.

Landed r241250 in meantime. Thank you for reviewing my patch!

Kind regards, Charlie.

[• chatur01]

In D10873#197976, @MatzeB wrote:

In my experience I also often have cases where n graphs are generated but I am only interested in one of them which forces me to close n-1 graph viewers.

I've just noticed there's a -filter-view-dags=name_of_BB_you_want_to_see that solves this problem. Still get random temp names for the dot file though.

include/llvm/Support/GraphWriter.h

 void ViewGraph(const GraphType &G, const Twine &Name,
                bool ShortNames = false, const Twine &Title = "",
                GraphProgram::Name Program = GraphProgram::DOT) {
   std::string Filename = llvm::WriteGraph(G, Name, ShortNames, Title);
 
   if (Filename.empty())
     return;
 
-  DisplayGraph(Filename, true, Program);
+  DisplayGraph(Filename, false, Program);
 }
 
 } // End llvm namespace
 
 #endif

lib/Support/GraphWriter.cpp

case GraphProgram::CIRCO:
     return "circo";
   }
   llvm_unreachable("bad kind");
 }
 
 bool llvm::DisplayGraph(StringRef FilenameRef, bool wait,
                         GraphProgram::Name program) {
   std::string Filename = FilenameRef;
-  wait &= !ViewBackground;
   std::string ErrMsg;
   std::string ViewerPath;
   GraphSession S;
 
 #ifdef __APPLE__
+  wait &= !ViewBackground;
   if (S.TryFindProgram("open", ViewerPath)) {
     std::vector<const char *> args;
     args.push_back(ViewerPath.c_str());
     if (wait)
       args.push_back("-W");
     args.push_back(Filename.c_str());
     args.push_back(nullptr);
     errs() << "Trying 'open' program... ";