[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Zequan Wu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGab8f622c79b2: [DebugInfo] Fix file path separator when 
targeting windows. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/debug-info-slash.c
  clang/test/Driver/cl-outputs.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,6 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
 PathRef = PathStore;
   }
 
Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -294,3 +294,12 @@
 // RUN: %clang_cl /P /Fifoo.x /obar.x -### -- %s 2>&1 | FileCheck -check-prefix=FioRACE2 %s
 // FioRACE2: "-E"
 // FioRACE2: "-o" "foo.x"
+
+// RUN: %clang_cl /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=ABSOLUTE_OBJPATH %s
+// ABSOLUTE_OBJPATH: "-object-file-name={{.*}}a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH1 %s
+// RELATIVE_OBJPATH1: "-object-file-name=a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Fofoo/a.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH2 %s
+// RELATIVE_OBJPATH2: "-object-file-name=foo\\a.obj"
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -563,6 +563,16 @@
 // Make the path absolute in the debug infos like MSVC does.
 llvm::sys::fs::make_absolute(ObjFileNameForDebug);
   }
+  // If the object file name is a relative path, then always use Windows
+  // backslash style as -object-file-name is used for embedding object file path
+  // in codeview and it can only be generated when targeting on Windows.
+  // Otherwise, just use native absolute path.
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native
+  : llvm::sys::path::Style::windows_backslash;
+  llvm::sys::path::remove_dots(ObjFileNameForDebug, /*remove_dot_dot=*/true,
+   Style);
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-object-file-name=") + ObjFileNameForDebug));
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the 

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

hans wrote:
> zequanwu wrote:
> > hans wrote:
> > > zequanwu wrote:
> > > > hans wrote:
> > > > > Won't the code above (line 580) make many filenames absolute and 
> > > > > cause us to use native slashes even when we want backslashes?
> > > > > 
> > > > > This would also need a test.
> > > > > Won't the code above (line 580) make many filenames absolute and 
> > > > > cause us to use native slashes even when we want backslashes?
> > > > Yes, I don't think we should change anything if it's converted to an 
> > > > absolute path.
> > > > 
> > > > Only if the `-fdebug-compilation-dir` is not given or is an absolute 
> > > > path, the path in `/Fo` will be converted to absolute path. Users can 
> > > > avoid the path being converted to absolute path by passing a relative 
> > > > path to `-fdebug-compilation-dir`, just like what we do in chrome build 
> > > > (`-fdebug-compilation-dir=.`).
> > > > 
> > > > Added a test.
> > > Apologies for all the questions, but I find the logic a bit complex 
> > > still. The test cases are helpful though.
> > > 
> > > Could we simplify by just always using Windows slashes here, regardless 
> > > of `-fdebug-compilation-dir` etc.?
> > The object file path will be converted to absolute path depending on 
> > whether `-fdebug-compilation-dir` is given or whether it's absolute path. I 
> > don't think we want to change a native absolute path of object file to use 
> > Windows slashes. Or this could happen on linux: `/usr/local/src/a.obj` -> 
> > `\usr\local\src\a.obj`. So, we only use windows slashes when it's not an 
> > absolute path here.
> Thanks, that makes sense. Can you add a comment with that explanation?
Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 514287.
zequanwu marked an inline comment as done.
zequanwu added a comment.

Add comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/debug-info-slash.c
  clang/test/Driver/cl-outputs.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,6 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
 PathRef = PathStore;
   }
 
Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -294,3 +294,12 @@
 // RUN: %clang_cl /P /Fifoo.x /obar.x -### -- %s 2>&1 | FileCheck -check-prefix=FioRACE2 %s
 // FioRACE2: "-E"
 // FioRACE2: "-o" "foo.x"
+
+// RUN: %clang_cl /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=ABSOLUTE_OBJPATH %s
+// ABSOLUTE_OBJPATH: "-object-file-name={{.*}}a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH1 %s
+// RELATIVE_OBJPATH1: "-object-file-name=a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Fofoo/a.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH2 %s
+// RELATIVE_OBJPATH2: "-object-file-name=foo\\a.obj"
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -579,6 +579,16 @@
 // Make the path absolute in the debug infos like MSVC does.
 llvm::sys::fs::make_absolute(ObjFileNameForDebug);
   }
+  // If the object file name is a relative path, then always use Windows
+  // backslash style as -object-file-name is used for embedding object file path
+  // in codeview and it can only be generated when targeting on Windows.
+  // Otherwise, just use native absolute path.
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native
+  : llvm::sys::path::Style::windows_backslash;
+  llvm::sys::path::remove_dots(ObjFileNameForDebug, /*remove_dot_dot=*/true,
+   Style);
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-object-file-name=") + ObjFileNameForDebug));
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the module name for
@@ -560,7 +567,6 @@
   }
 
   llvm::dwarf::SourceLanguage LangTag;
-  const LangOptions  = 

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

zequanwu wrote:
> hans wrote:
> > zequanwu wrote:
> > > hans wrote:
> > > > Won't the code above (line 580) make many filenames absolute and cause 
> > > > us to use native slashes even when we want backslashes?
> > > > 
> > > > This would also need a test.
> > > > Won't the code above (line 580) make many filenames absolute and cause 
> > > > us to use native slashes even when we want backslashes?
> > > Yes, I don't think we should change anything if it's converted to an 
> > > absolute path.
> > > 
> > > Only if the `-fdebug-compilation-dir` is not given or is an absolute 
> > > path, the path in `/Fo` will be converted to absolute path. Users can 
> > > avoid the path being converted to absolute path by passing a relative 
> > > path to `-fdebug-compilation-dir`, just like what we do in chrome build 
> > > (`-fdebug-compilation-dir=.`).
> > > 
> > > Added a test.
> > Apologies for all the questions, but I find the logic a bit complex still. 
> > The test cases are helpful though.
> > 
> > Could we simplify by just always using Windows slashes here, regardless of 
> > `-fdebug-compilation-dir` etc.?
> The object file path will be converted to absolute path depending on whether 
> `-fdebug-compilation-dir` is given or whether it's absolute path. I don't 
> think we want to change a native absolute path of object file to use Windows 
> slashes. Or this could happen on linux: `/usr/local/src/a.obj` -> 
> `\usr\local\src\a.obj`. So, we only use windows slashes when it's not an 
> absolute path here.
Thanks, that makes sense. Can you add a comment with that explanation?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

hans wrote:
> zequanwu wrote:
> > hans wrote:
> > > Won't the code above (line 580) make many filenames absolute and cause us 
> > > to use native slashes even when we want backslashes?
> > > 
> > > This would also need a test.
> > > Won't the code above (line 580) make many filenames absolute and cause us 
> > > to use native slashes even when we want backslashes?
> > Yes, I don't think we should change anything if it's converted to an 
> > absolute path.
> > 
> > Only if the `-fdebug-compilation-dir` is not given or is an absolute path, 
> > the path in `/Fo` will be converted to absolute path. Users can avoid the 
> > path being converted to absolute path by passing a relative path to 
> > `-fdebug-compilation-dir`, just like what we do in chrome build 
> > (`-fdebug-compilation-dir=.`).
> > 
> > Added a test.
> Apologies for all the questions, but I find the logic a bit complex still. 
> The test cases are helpful though.
> 
> Could we simplify by just always using Windows slashes here, regardless of 
> `-fdebug-compilation-dir` etc.?
The object file path will be converted to absolute path depending on whether 
`-fdebug-compilation-dir` is given or whether it's absolute path. I don't think 
we want to change a native absolute path of object file to use Windows slashes. 
Or this could happen on linux: `/usr/local/src/a.obj` -> 
`\usr\local\src\a.obj`. So, we only use windows slashes when it's not an 
absolute path here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-17 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

zequanwu wrote:
> hans wrote:
> > Won't the code above (line 580) make many filenames absolute and cause us 
> > to use native slashes even when we want backslashes?
> > 
> > This would also need a test.
> > Won't the code above (line 580) make many filenames absolute and cause us 
> > to use native slashes even when we want backslashes?
> Yes, I don't think we should change anything if it's converted to an absolute 
> path.
> 
> Only if the `-fdebug-compilation-dir` is not given or is an absolute path, 
> the path in `/Fo` will be converted to absolute path. Users can avoid the 
> path being converted to absolute path by passing a relative path to 
> `-fdebug-compilation-dir`, just like what we do in chrome build 
> (`-fdebug-compilation-dir=.`).
> 
> Added a test.
Apologies for all the questions, but I find the logic a bit complex still. The 
test cases are helpful though.

Could we simplify by just always using Windows slashes here, regardless of 
`-fdebug-compilation-dir` etc.?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-14 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

hans wrote:
> Won't the code above (line 580) make many filenames absolute and cause us to 
> use native slashes even when we want backslashes?
> 
> This would also need a test.
> Won't the code above (line 580) make many filenames absolute and cause us to 
> use native slashes even when we want backslashes?
Yes, I don't think we should change anything if it's converted to an absolute 
path.

Only if the `-fdebug-compilation-dir` is not given or is an absolute path, the 
path in `/Fo` will be converted to absolute path. Users can avoid the path 
being converted to absolute path by passing a relative path to 
`-fdebug-compilation-dir`, just like what we do in chrome build 
(`-fdebug-compilation-dir=.`).

Added a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-14 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 513611.
zequanwu added a comment.

Added clang-cl option test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/debug-info-slash.c
  clang/test/Driver/cl-outputs.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,6 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
 PathRef = PathStore;
   }
 
Index: clang/test/Driver/cl-outputs.c
===
--- clang/test/Driver/cl-outputs.c
+++ clang/test/Driver/cl-outputs.c
@@ -294,3 +294,12 @@
 // RUN: %clang_cl /P /Fifoo.x /obar.x -### -- %s 2>&1 | FileCheck -check-prefix=FioRACE2 %s
 // FioRACE2: "-E"
 // FioRACE2: "-o" "foo.x"
+
+// RUN: %clang_cl /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=ABSOLUTE_OBJPATH %s
+// ABSOLUTE_OBJPATH: "-object-file-name={{.*}}a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Foa.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH1 %s
+// RELATIVE_OBJPATH1: "-object-file-name=a.obj"
+
+// RUN: %clang_cl -fdebug-compilation-dir=. /Z7 /Fofoo/a.obj -### -- %s 2>&1 | FileCheck -check-prefix=RELATIVE_OBJPATH2 %s
+// RELATIVE_OBJPATH2: "-object-file-name=foo\\a.obj"
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -579,6 +579,12 @@
 // Make the path absolute in the debug infos like MSVC does.
 llvm::sys::fs::make_absolute(ObjFileNameForDebug);
   }
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native
+  : llvm::sys::path::Style::windows_backslash;
+  llvm::sys::path::remove_dots(ObjFileNameForDebug, /*remove_dot_dot=*/true,
+   Style);
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-object-file-name=") + ObjFileNameForDebug));
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the module name for
@@ -560,7 +567,6 @@
   }
 
   llvm::dwarf::SourceLanguage LangTag;
-  const LangOptions  = CGM.getLangOpts();
   if (LO.CPlusPlus) {
 if (LO.ObjC)
   LangTag = llvm::dwarf::DW_LANG_ObjC_plus_plus;
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-14 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:583
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native

Won't the code above (line 580) make many filenames absolute and cause us to 
use native slashes even when we want backslashes?

This would also need a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-13 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 513252.
zequanwu added a comment.
Herald added a subscriber: MaskRay.

- Move remove_dots to clang driver.
- Revert changes to llc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/debug-info-slash.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,6 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
 PathRef = PathStore;
   }
 
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g 
%s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g 
%s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -579,6 +579,12 @@
 // Make the path absolute in the debug infos like MSVC does.
 llvm::sys::fs::make_absolute(ObjFileNameForDebug);
   }
+  llvm::sys::path::Style Style =
+  llvm::sys::path::is_absolute(ObjFileNameForDebug)
+  ? llvm::sys::path::Style::native
+  : llvm::sys::path::Style::windows_backslash;
+  llvm::sys::path::remove_dots(ObjFileNameForDebug, /*remove_dot_dot=*/true,
+   Style);
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-object-file-name=") + ObjFileNameForDebug));
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the module name for
@@ -560,7 +567,6 @@
   }
 
   llvm::dwarf::SourceLanguage LangTag;
-  const LangOptions  = CGM.getLangOpts();
   if (LO.CPlusPlus) {
 if (LO.ObjC)
   LangTag = llvm::dwarf::DW_LANG_ObjC_plus_plus;
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -479,9 +479,9 @@
   /// The seed used by the randomize structure layout feature.
   std::string RandstructSeed;
 
-  /// Indicates whether the __FILE__ macro should use the target's
-  /// platform-specific file separator or whether it should use the build
-  /// environment's platform-specific file separator.
+  /// Indicates whether to use target's platform-specific file separator when
+  /// __FILE__ macro is used and when concatenating filename with directory or
+  /// to use build environment environment's platform-specific file separator.
   ///
   /// The plaform-specific path separator is the backslash(\) for Windows and
   /// forward slash (/) elsewhere.


Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-12 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added subscribers: thieta, saudi.
aganea added a comment.

+ @saudi @thieta


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-12 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D147256#4262002 , @zequanwu wrote:

> In D147256#4261949 , @hans wrote:
>
>>> Well, MSVC cl removes redundant dots so we shouldn't remove 
>>> llvm::sys::path::remove_dots.
>>
>> Could we do the `remove_dots` on the Clang side, where we can decide based 
>> on the LangOpts?
>
> Yes, we can do that. Is there a reason to favor there over here? At here, the 
> object path in llc output can also benefits from `remove_dots`, not just 
> clang.

The benefit is that if we do it in Clang, we can just look at the LangOpts and 
don't have to add a new target option, command line flag etc.

For llc, since it's an internal tool, I think it would be fine to just use the 
name as given.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-12 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D147256#4261949 , @hans wrote:

>> Well, MSVC cl removes redundant dots so we shouldn't remove 
>> llvm::sys::path::remove_dots.
>
> Could we do the `remove_dots` on the Clang side, where we can decide based on 
> the LangOpts?

Yes, we can do that. Is there a reason to favor there over here? At here, the 
object path in llc output can also benefits from `remove_dots`, not just clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-12 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

> Well, MSVC cl removes redundant dots so we shouldn't remove 
> llvm::sys::path::remove_dots.

Could we do the `remove_dots` on the Clang side, where we can decide based on 
the LangOpts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-11 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D147256#4258419 , @zequanwu wrote:

> In D147256#4257797 , @hans wrote:
>
>> In D147256#4249527 , @zequanwu 
>> wrote:
>>
>>> - Add a `-use-target-path-separator` flag for llc.
>>> - Add test for llc with that flag.
>>
>> But where does `TM.Options.ObjectFilenameForDebug` come from? Presumably it 
>> comes from Clang at some point, so is there any chance we can fix it "at the 
>> source" instead?
>
> `TM.Options.ObjectFilenameForDebug` either comes from llc's `-o` or 
> clang-cl's `object-file-name=` which is translated from `/Fo[ObjectFileName]`.
>
> For Chromium, the `/Fo[ObjectFileName]` we pass to clang-cl is the same when 
> building on Windows and targeting Windows from Linux. The problem comes 
> `llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);` in 
> `CodeViewDebug.cpp`. That function always convert the path to use host's path 
> separator. And I don't think we can write a `remove_dots` function that 
> doesn't change path separator, because `\` can only be used as path separator 
> on Windows but is a valid path character on Linux.
>
> Or we could just not use `llvm::sys::path::remove_dots`, use the user's input 
> as it is for object file path.

Well, MSVC cl removes redundant dots so we shouldn't remove 
`llvm::sys::path::remove_dots`. 
Generally, I think it's okay to use target path separator for object file path 
in codeview when user input is not an absolute path, because we can only 
generate codeview when targeting on windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-11 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D147256#4257797 , @hans wrote:

> In D147256#4249527 , @zequanwu 
> wrote:
>
>> - Add a `-use-target-path-separator` flag for llc.
>> - Add test for llc with that flag.
>
> But where does `TM.Options.ObjectFilenameForDebug` come from? Presumably it 
> comes from Clang at some point, so is there any chance we can fix it "at the 
> source" instead?

`TM.Options.ObjectFilenameForDebug` either comes from llc's `-o` or clang-cl's 
`object-file-name=` which is translated from `/Fo[ObjectFileName]`.

For Chromium, the `/Fo[ObjectFileName]` we pass to clang-cl is the same when 
building on Windows and targeting Windows from Linux. The problem comes 
`llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);` in 
`CodeViewDebug.cpp`. That function always convert the path to use host's path 
separator. And I don't think we can write a `remove_dots` function that doesn't 
change path separator, because `\` can only be used as path separator on 
Windows but is a valid path character on Linux.

Or we could just not use `llvm::sys::path::remove_dots`, use the user's input 
as it is for object file path.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-11 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D147256#4249527 , @zequanwu wrote:

> - Add a `-use-target-path-separator` flag for llc.
> - Add test for llc with that flag.

But where does `TM.Options.ObjectFilenameForDebug` come from? Presumably it 
comes from Clang at some point, so is there any chance we can fix it "at the 
source" instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-06 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 511496.
zequanwu added a comment.

- Add a `-use-target-path-separator` flag for llc.
- Add test for llc with that flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-slash.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/test/DebugInfo/COFF/use-target-path-separator.ll

Index: llvm/test/DebugInfo/COFF/use-target-path-separator.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/COFF/use-target-path-separator.ll
@@ -0,0 +1,35 @@
+; RUN: rm -rf %t-dir
+; RUN: mkdir -p %t-dir/subdir
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc -use-target-path-separator %s -o subdir/win.obj
+; RUN: llvm-readobj --codeview subdir/win.obj | FileCheck --check-prefix=WIN %s
+
+; WIN: ObjectName: subdir\win.obj
+
+; ModuleID = 'D:\src\scopes\foo.cpp'
+source_filename = "D:\5Csrc\5Cscopes\5Cfoo.cpp"
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-pc-windows-msvc19.0.23918"
+
+define i32 @"?foo@@YAHXZ"() !dbg !10 {
+entry:
+  ret i32 42, !dbg !14
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 4.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+; One .debug$S section should contain an S_COMPILE3 record that identifies the
+; source language and the version of the compiler based on the DICompileUnit.
+!1 = !DIFile(filename: "D:\5Csrc\5Cscopes\5Cfoo.cpp", directory: "D:\5Csrc\5Cscopes\5Cclang")
+!2 = !{}
+!7 = !{i32 2, !"CodeView", i32 1}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{!"clang version 4.0.0 "}
+!10 = distinct !DISubprogram(name: "foo", linkageName: "\01?foo@@YAHXZ", scope: !1, file: !1, line: 1, type: !11, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13}
+!13 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!14 = !DILocation(line: 2, scope: !10)
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -104,6 +104,7 @@
 CGOPT(unsigned, AlignLoops)
 CGOPT(bool, JMCInstrument)
 CGOPT(bool, XCOFFReadOnlyPointers)
+CGOPT(bool, UseTargetPathSeparator)
 
 codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() {
 #define CGBINDOPT(NAME)\
@@ -486,6 +487,14 @@
   cl::init(false));
   CGBINDOPT(XCOFFReadOnlyPointers);
 
+  static cl::opt UseTargetPathSeparator(
+  "use-target-path-separator",
+  cl::desc(
+  "Use target's platform-specific file separator when concatenating "
+  "filename with directory."),
+  cl::init(false));
+  CGBINDOPT(UseTargetPathSeparator);
+
 #undef CGBINDOPT
 
   mc::RegisterMCTargetOptionsFlags();
@@ -563,6 +572,7 @@
   Options.LoopAlignment = getAlignLoops();
   Options.JMCInstrument = getJMCInstrument();
   Options.XCOFFReadOnlyPointers = getXCOFFReadOnlyPointers();
+  Options.UseTargetPathSeparator = getUseTargetPathSeparator();
 
   Options.MCOptions = mc::InitMCTargetOptionsFromFlags();
 
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,15 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
+llvm::sys::path::Style Style =
+(!llvm::sys::path::is_absolute(PathRef,
+   llvm::sys::path::Style::posix) &&
+ Asm->TM.Options.UseTargetPathSeparator)
+? (Asm->TM.getTargetTriple().isOSWindows()
+   ? llvm::sys::path::Style::windows_backslash
+   : llvm::sys::path::Style::posix)
+: llvm::sys::path::Style::native;
+llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true, Style);
 PathRef = PathStore;
   }
 
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,7 +127,8 @@
 : UnsafeFPMath(false), NoInfsFPMath(false), NoNaNsFPMath(false),
 

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-05 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

An LLVM code change should be testable on its own; this has it tested by Clang.
I think you need a new command-line option to set 
TargetOptions::UseTargetPathSeparator e.g. via llvm-mc. Other TargetOptions are 
handled this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-04-05 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 58.
zequanwu added a comment.
Herald added a subscriber: ormris.

- Pass `LangOptions.UseTargetPathSeparator` down to TargetOptions to use host

path separator when it's set.

- Add a test case. The problem is when if we are targeting to linux, clang

won't generate codeview debuginfo with `-gcodeview`. So, I only add the test to
target on windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-slash.c
  clang/test/CodeGen/use-target-path-separator.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,15 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
+llvm::sys::path::Style Style =
+(!llvm::sys::path::is_absolute(PathRef,
+   llvm::sys::path::Style::posix) &&
+ Asm->TM.Options.UseTargetPathSeparator)
+? (Asm->TM.getTargetTriple().isOSWindows()
+   ? llvm::sys::path::Style::windows_backslash
+   : llvm::sys::path::Style::posix)
+: llvm::sys::path::Style::native;
+llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true, Style);
 PathRef = PathStore;
   }
 
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -127,7 +127,8 @@
 : UnsafeFPMath(false), NoInfsFPMath(false), NoNaNsFPMath(false),
   NoTrappingFPMath(true), NoSignedZerosFPMath(false),
   ApproxFuncFPMath(false), EnableAIXExtendedAltivecABI(false),
-  HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
+  HonorSignDependentRoundingFPMathOption(false),
+  UseTargetPathSeparator(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
   EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
   DisableIntegratedAS(false), RelaxELFRelocations(true),
@@ -206,6 +207,11 @@
 unsigned HonorSignDependentRoundingFPMathOption : 1;
 bool HonorSignDependentRoundingFPMath() const;
 
+/// UseTargetPathSeparator - This Indicates whether to use target's
+/// platform-specific file separator when concatenating filename with
+/// directory.
+unsigned UseTargetPathSeparator : 1;
+
 /// NoZerosInBSS - By default some codegens place zero-initialized data to
 /// .bss section. This flag disables such behaviour (necessary, e.g. for
 /// crt*.o compiling).
Index: clang/test/CodeGen/use-target-path-separator.c
===
--- /dev/null
+++ clang/test/CodeGen/use-target-path-separator.c
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir/subdir
+// RUN: cd %t-dir
+// RUN: %clang_cl --target=x86_64-windows-msvc -ffile-reproducible /c /Z7 -fdebug-compilation-dir=. /Fosubdir/win.obj -- %s
+// RUN: llvm-readobj --codeview subdir/win.obj | FileCheck --check-prefix=WIN %s
+
+int main (void) {
+  return 0;
+}
+
+// WIN: ObjectName: subdir\win.obj
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-31 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D147256#4237099 , @probinson wrote:

> I think we cannot be 100% sure about source paths in a cross-compile 
> situation. Cross-compiling on platform A targeting platform B does not mean 
> your sources and debugger UI are on platform B. My users keep source and 
> debugger UI on platform A, debugging target B remotely. We need to preserve 
> the host pathnames. It is not clear to me that this patch does so.

I believe the idea is to preserve host pathnames as long as 
`LangOptions.UseTargetPathSeparator` is unset. But I see that might not be the 
case for `CodeViewDebug::emitObjName()` in the current version of the patch. 
Could we make clang handle that path instead depending on the flag?




Comment at: llvm/test/DebugInfo/COFF/build-info.ll:18
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s 
--check-prefix=OBJ

zequanwu wrote:
> hans wrote:
> > Does this write the .o file into the test directory? I don't think that's 
> > allowed (it may be read-only). But the test could create another 
> > subdirectory under `%t-dir`.
> It writes the .o file into the parent directory of the temporary dir %t-dir. 
> It will just be the directory path of a normal `%t`, not the source test 
> directory. The reason I'm not using `%t-dir/build-info.o` in the parent dir 
> is because it will be translated into an absolute address. That will remain 
> unchanged in ObjectName.
Right, I'm just thinking that we can't be sure that we can (or should) write 
files into the parent of `%t-dir`.

Would it work if you instead create `%t-dir/subdir`, `cd` into that, and then 
do `-o ../build-info.o`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-31 Thread Paul Robinson via Phabricator via cfe-commits
probinson added subscribers: debug-info, probinson.
probinson added a comment.

I think we cannot be 100% sure about source paths in a cross-compile situation. 
Cross-compiling on platform A targeting platform B does not mean your sources 
and debugger UI are on platform B. My users keep source and debugger UI on 
platform A, debugging target B remotely. We need to preserve the host 
pathnames. It is not clear to me that this patch does so.




Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:787
 
   StringRef PathRef(Asm->TM.Options.ObjectFilenameForDebug);
   llvm::SmallString<256> PathStore(PathRef);

zequanwu wrote:
> hans wrote:
> > This handles codeview. Does anything need to be done for dwarf on windows? 
> > mstorsjo might have input on that.
> It looks like `TM.Options.ObjectFilenameForDebug` is only used for codeview. 
> I guess dwarf doesn't store the object file path.
Right, DWARF only stores the source path.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-31 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added a comment.

In D147256#4236522 , @hans wrote:

> Thanks for working on this!
>
> The `-ffile-reproducible` flag name refers to making `#file` directives 
> reproducible, but `LangOptions.UseTargetPathSeparator` sounds a lot broader 
> :) I don't know what others think, but it would be nice to not have to 
> introduce any more flags at least.

Oh, I was already using `LangOptions.UseTargetPathSeparator`. Updated the 
comment on UseTargetPathSeparator.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:544
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);

hans wrote:
> Do we want to fix absolute filenames too?
> I can see arguments for and against:
> - Using what the user provided makes sense
> - Some build systems might use absolute paths for everything. But on the 
> other hand such builds have larger determinism problems (including the full 
> paths).
> So the current decision probably makes sense.
Yeah. If it's already absolute filename, it will just use that one user 
provided. 



Comment at: clang/test/CodeGen/debug-info-slash.c:5
+
+// WIN:   !DIFile(filename: "{{.*}}clang/test/CodeGen\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}clang/test/CodeGen/debug-info-slash.c"

hans wrote:
> Does the test runner write the 'clang/test/CodeGen' path with forward slashes 
> also on Windows?
No, it uses `clang\\test\\CodeGen`. Removed this part.



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:787
 
   StringRef PathRef(Asm->TM.Options.ObjectFilenameForDebug);
   llvm::SmallString<256> PathStore(PathRef);

hans wrote:
> This handles codeview. Does anything need to be done for dwarf on windows? 
> mstorsjo might have input on that.
It looks like `TM.Options.ObjectFilenameForDebug` is only used for codeview. I 
guess dwarf doesn't store the object file path.



Comment at: llvm/test/DebugInfo/COFF/build-info.ll:18
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s 
--check-prefix=OBJ

hans wrote:
> Does this write the .o file into the test directory? I don't think that's 
> allowed (it may be read-only). But the test could create another subdirectory 
> under `%t-dir`.
It writes the .o file into the parent directory of the temporary dir %t-dir. It 
will just be the directory path of a normal `%t`, not the source test 
directory. The reason I'm not using `%t-dir/build-info.o` in the parent dir is 
because it will be translated into an absolute address. That will remain 
unchanged in ObjectName.



Comment at: llvm/test/DebugInfo/COFF/build-info.ll:21
+
+; OBJ: ObjectName: ..\build-info.o
+

hans wrote:
> But in the `llc` invocation, the user wrote a relative path with a forward 
> slash. What behavior do we want? What should happen if there are more then 
> one slash - I think remove_dots just works on the first one?
Yeah, the input uses forward slash but we convert it into backslash. When there 
are multiple slashs, they will all be converted into backslashs, which is done 
by `llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true, 
llvm::sys::path::Style::windows_backslash);`. `remove_dots` not only remove 
redundant dots but also canonicalize path separator based on the style.

I think ideally we want just take what user provided as the ObjectName with the 
redundant dots removed. But `remove_dots` always canonicalizes the path 
separator based on the style. I don't find any function that doesn't 
canonicalize the path. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-31 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 510057.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-slash.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/test/DebugInfo/COFF/build-info.ll

Index: llvm/test/DebugInfo/COFF/build-info.ll
===
--- llvm/test/DebugInfo/COFF/build-info.ll
+++ llvm/test/DebugInfo/COFF/build-info.ll
@@ -10,6 +10,16 @@
 
 ; CHECK: {{.*}} | S_BUILDINFO [size = 8] BuildId = `[[INFO_IDX]]`
 
+; Test path is canonicalized to windows backslash style when output object file
+; name is not starting with '/'.
+; RUN: rm -rf %t-dir
+; RUN: mkdir %t-dir
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s --check-prefix=OBJ
+
+; OBJ: ObjectName: ..\build-info.o
+
 ; ModuleID = 'D:\src\scopes\foo.cpp'
 source_filename = "D:\5Csrc\5Cscopes\5Cfoo.cpp"
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,11 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
+llvm::sys::path::Style Style =
+llvm::sys::path::is_absolute(PathRef, llvm::sys::path::Style::posix)
+? llvm::sys::path::Style::posix
+: llvm::sys::path::Style::windows_backslash;
+llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true, Style);
 PathRef = PathStore;
   }
 
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g %s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}/debug-info-slash.c"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the module name for
@@ -560,7 +567,6 @@
   }
 
   llvm::dwarf::SourceLanguage LangTag;
-  const LangOptions  = CGM.getLangOpts();
   if (LO.CPlusPlus) {
 if (LO.ObjC)
   LangTag = llvm::dwarf::DW_LANG_ObjC_plus_plus;
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -479,9 +479,9 @@
   /// The seed used by the randomize structure layout feature.
   std::string RandstructSeed;
 
-  /// Indicates whether the __FILE__ macro should use the target's
-  /// platform-specific file separator or whether it should use the build
-  /// environment's platform-specific file separator.
+  /// Indicates whether to use target's platform-specific file separator when
+  /// __FILE__ macro is used and when concatenating filename with directory or
+  

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-31 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Thanks for working on this!

The `-ffile-reproducible` flag name refers to making `#file` directives 
reproducible, but `LangOptions.UseTargetPathSeparator` sounds a lot broader :) 
I don't know what others think, but it would be nice to not have to introduce 
any more flags at least.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:544
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);

Do we want to fix absolute filenames too?
I can see arguments for and against:
- Using what the user provided makes sense
- Some build systems might use absolute paths for everything. But on the other 
hand such builds have larger determinism problems (including the full paths).
So the current decision probably makes sense.



Comment at: clang/test/CodeGen/debug-info-slash.c:5
+
+// WIN:   !DIFile(filename: "{{.*}}clang/test/CodeGen\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}clang/test/CodeGen/debug-info-slash.c"

Does the test runner write the 'clang/test/CodeGen' path with forward slashes 
also on Windows?



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:787
 
   StringRef PathRef(Asm->TM.Options.ObjectFilenameForDebug);
   llvm::SmallString<256> PathStore(PathRef);

This handles codeview. Does anything need to be done for dwarf on windows? 
mstorsjo might have input on that.



Comment at: llvm/test/DebugInfo/COFF/build-info.ll:13
 
+; Test path is canalized to windows backslash style when output object file 
name
+; is not starting with '/'.

s/canalized/canonicalized/



Comment at: llvm/test/DebugInfo/COFF/build-info.ll:18
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s 
--check-prefix=OBJ

Does this write the .o file into the test directory? I don't think that's 
allowed (it may be read-only). But the test could create another subdirectory 
under `%t-dir`.



Comment at: llvm/test/DebugInfo/COFF/build-info.ll:21
+
+; OBJ: ObjectName: ..\build-info.o
+

But in the `llc` invocation, the user wrote a relative path with a forward 
slash. What behavior do we want? What should happen if there are more then one 
slash - I think remove_dots just works on the first one?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

2023-03-30 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu created this revision.
zequanwu added reviewers: hans, aaron.ballman, ayzhao, aganea.
Herald added a subscriber: hiraditya.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This fixes two problems:

1. When crossing compiling for windows on linux, source file path in debug info

is concatenated with directory by host native separator ('/'). For windows
local build, they are concatenated by '\'. This causes non-determinism bug.

The solution here is to let `-ffile-reproducible` to control if we should use
host native separator or not.

2. Objectfile path in CodeView also uses host native separator when generated.

Make it always to use `\` unless the it's absolute path in posix style.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147256

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-slash.c
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/test/DebugInfo/COFF/build-info.ll


Index: llvm/test/DebugInfo/COFF/build-info.ll
===
--- llvm/test/DebugInfo/COFF/build-info.ll
+++ llvm/test/DebugInfo/COFF/build-info.ll
@@ -10,6 +10,16 @@
 
 ; CHECK: {{.*}} | S_BUILDINFO [size = 8] BuildId = `[[INFO_IDX]]`
 
+; Test path is canalized to windows backslash style when output object file 
name
+; is not starting with '/'.
+; RUN: rm -rf %t-dir
+; RUN: mkdir %t-dir
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s 
--check-prefix=OBJ
+
+; OBJ: ObjectName: ..\build-info.o
+
 ; ModuleID = 'D:\src\scopes\foo.cpp'
 source_filename = "D:\5Csrc\5Cscopes\5Cfoo.cpp"
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -791,7 +791,11 @@
 // Don't emit the filename if we're writing to stdout or to /dev/null.
 PathRef = {};
   } else {
-llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
+llvm::sys::path::Style Style =
+llvm::sys::path::is_absolute(PathRef, llvm::sys::path::Style::posix)
+? llvm::sys::path::Style::posix
+: llvm::sys::path::Style::windows_backslash;
+llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true, Style);
 PathRef = PathStore;
   }
 
Index: clang/test/CodeGen/debug-info-slash.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-slash.c
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-pc-win32  -ffile-reproducible -emit-llvm -S -g 
%s -o - | FileCheck --check-prefix=WIN %s
+// RUN: %clang -target x86_64-linux-gnu  -ffile-reproducible -emit-llvm -S -g 
%s -o - | FileCheck --check-prefix=LINUX %s
+int main() { return 0; }
+
+// WIN:   !DIFile(filename: "{{.*}}clang/test/CodeGen\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}clang/test/CodeGen/debug-info-slash.c"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -528,6 +528,7 @@
   // Get absolute path name.
   SourceManager  = CGM.getContext().getSourceManager();
   auto  = CGM.getCodeGenOpts();
+  const LangOptions  = CGM.getLangOpts();
   std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
@@ -542,9 +543,15 @@
 MainFileDir = std::string(MainFile->getDir().getName());
 if (!llvm::sys::path::is_absolute(MainFileName)) {
   llvm::SmallString<1024> MainFileDirSS(MainFileDir);
-  llvm::sys::path::append(MainFileDirSS, MainFileName);
-  MainFileName =
-  std::string(llvm::sys::path::remove_leading_dotslash(MainFileDirSS));
+  llvm::sys::path::Style Style =
+  LO.UseTargetPathSeparator
+  ? (CGM.getTarget().getTriple().isOSWindows()
+ ? llvm::sys::path::Style::windows_backslash
+ : llvm::sys::path::Style::posix)
+  : llvm::sys::path::Style::native;
+  llvm::sys::path::append(MainFileDirSS, Style, MainFileName);
+  MainFileName = std::string(
+  llvm::sys::path::remove_leading_dotslash(MainFileDirSS, Style));
 }
 // If the main file name provided is identical to the input file name, and
 // if the input file is a preprocessed source, use the module name for
@@ -560,7 +567,6 @@
   }
 
   llvm::dwarf::SourceLanguage LangTag;
-  const LangOptions  = CGM.getLangOpts();
   if (LO.CPlusPlus) {
 if (LO.ObjC)
   LangTag = llvm::dwarf::DW_LANG_ObjC_plus_plus;


Index: llvm/test/DebugInfo/COFF/build-info.ll