bitsandbytes icon indicating copy to clipboard operation
bitsandbytes copied to clipboard

CPU/XPU: disable torch.compile if g++ is not available

Open Xia-Weiwen opened this issue 1 year ago • 11 comments

torch.compile requires g++. On platforms like Windows, g++ is not available. In that case, torch.compile is disabled to avoid runtime errors. With this patch, the CPU/XPU backend works on Windows.

I have validated CPU backend with the patch by running UTs for CPU backend in test_functional.py and a real model (OPT-1.3B) on my local machine (Windows 11 Enterprise, 12th Gen Intel(R) Core(TM) i7-1270P 2.20 GHz, 16.0 GB RAM).

Xia-Weiwen avatar Jun 17 '24 08:06 Xia-Weiwen

Hi @Titus-von-Koeller Could you please review? Thanks. Looks like the CI failure is about ROCM. I am not sure if the failure is caused by this PR.

Xia-Weiwen avatar Jun 18 '24 05:06 Xia-Weiwen

Related: pytorch/pytorch#124245

I'm not sure what PyTorch release that's going to land in, but good news is that it looks like it'll eventually support MSVC on Windows and we can update this as appropriate. Until then, I think this will be OK.

matthewdouglas avatar Jun 19 '24 15:06 matthewdouglas

https://github.com/pytorch/pytorch/blob/main/torch/_inductor/cpp_builder.py#L123-L134 @Xia-Weiwen FYI.

xuhancn avatar Jun 19 '24 21:06 xuhancn

Hi @matthewdouglas Yes, it will still take some time to enable torch.compile on Windows, and the clang compiler is required. The contributor is @xuhancn . So, I would agree that it's better to disable it on Windows now and enable it later when everything is ready.

Xia-Weiwen avatar Jun 20 '24 03:06 Xia-Weiwen

Hi @matthewdouglas Yes, it will still take some time to enable torch.compile on Windows, and the clang compiler is required. The contributor is @xuhancn . So, I would agree that it's better to disable it on Windows now and enable it later when everything is ready.

Windows default compiler is msvc(cl.exe), please correct it. :)

xuhancn avatar Jun 20 '24 07:06 xuhancn

Hi @matthewdouglas Yes, it will still take some time to enable torch.compile on Windows, and the clang compiler is required. The contributor is @xuhancn . So, I would agree that it's better to disable it on Windows now and enable it later when everything is ready.

Windows default compiler is msvc(cl.exe), please correct it. :)

Thanks for the correction.

Xia-Weiwen avatar Jun 20 '24 08:06 Xia-Weiwen

Hi @matthewdouglas Yes, it will still take some time to enable torch.compile on Windows, and the clang compiler is required. The contributor is @xuhancn . So, I would agree that it's better to disable it on Windows now and enable it later when everything is ready.

Windows default compiler is msvc(cl.exe), please correct it. :)

Thanks for the correction.

@Xia-Weiwen does this still need correction or is this ready to merge? In the diff it still shows gcc for the compiler...

Titus-von-Koeller avatar Jun 21 '24 17:06 Titus-von-Koeller

@Titus-von-Koeller That's just a correction on comment about the compiler requirement for Windows (when we get there). It will use MSVC by default and not clang. I think it's OK to merge now as it is and my commentary here is just a note for future refinement.

matthewdouglas avatar Jun 21 '24 19:06 matthewdouglas

Hi @Titus-von-Koeller . This PR does not need correction. torch.compile is not ready on Windows yet. G++ is required for torch.compile now. So, this PR just disable it on Windows by checking if G++ is available. @matthewdouglas is correct. Thanks.

Xia-Weiwen avatar Jun 22 '24 08:06 Xia-Weiwen

Hi @Titus-von-Koeller Do you have more comments? Thanks

Xia-Weiwen avatar Jun 26 '24 00:06 Xia-Weiwen

Hallo @Titus-von-Koeller It's been a while since last update. Do you think there are any blocking issues preventing this PR merging? Thanks.

Xia-Weiwen avatar Jul 04 '24 03:07 Xia-Weiwen

@Xia-Weiwen Yes, everything is right the way it is then, thanks for the clarification!

Titus-von-Koeller avatar Jul 10 '24 07:07 Titus-von-Koeller

@Xia-Weiwen Yes, everything is right the way it is then, thanks for the clarification!

Thanks!

Xia-Weiwen avatar Jul 21 '24 23:07 Xia-Weiwen