Skip to content

Fix exceptions not being caught#1309

Closed
brunoalr wants to merge 3 commits intopytorch:masterfrom
PPC64:add-fexceptions
Closed

Fix exceptions not being caught#1309
brunoalr wants to merge 3 commits intopytorch:masterfrom
PPC64:add-fexceptions

Conversation

@brunoalr
Copy link

@brunoalr brunoalr commented Apr 20, 2017

Adding -fexceptions to both torch and pytorch C/C++ builds fixes tests
not passing in ppc64le and aarch64.

See https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_exceptions.html

Closes #1297

Adding -fexceptions to both torch and pytorch C/C++ builds fixes tests
not passing.

Closes pytorch#1297
@apaszke
Copy link
Contributor

apaszke commented Apr 20, 2017

@pytorchbot test this please

1 similar comment
@soumith
Copy link
Collaborator

soumith commented Apr 20, 2017

@pytorchbot test this please

@apaszke
Copy link
Contributor

apaszke commented Apr 20, 2017

The PR breaks CUDA builds (nvcc doesn't support -fexceptions)

@brunoalr
Copy link
Author

Right now I can't do a CUDA build. But the main idea should be just not to pass -fexceptions to nvcc in torch/lib. I will take a look at it.

@soumith
Copy link
Collaborator

soumith commented Apr 20, 2017

from what i get, you only need -fexceptions in setup.py. The other change is not needed. It's because we make our TH error handler to throw exceptions on the python extension side at runtime.
That would also fix the CUDA issue.

@brunoalr
Copy link
Author

@soumith I tested giving up the changes in build_all.sh and unfortunately that does not work. We have to pass the flag to both C and C++ builds.

@apaszke
Copy link
Contributor

apaszke commented Apr 20, 2017

@soumith The flag is only needed for files compiled with the C compiler, so it's needed for torch/lib too.

@soumith
Copy link
Collaborator

soumith commented Apr 24, 2017

@pytorchbot test this please

@brunoalr
Copy link
Author

@apaszke @soumith can you review this updated PR, please?

Bruno Rosa added 2 commits April 24, 2017 22:13
cd build/$1
BUILD_C_FLAGS=''
eval 'case '$1' in
'${NVCC_TARGETS//" "/" | "}') BUILD_C_FLAGS=$C_FLAGS;;

This comment was marked as off-topic.

This comment was marked as off-topic.

@soumith
Copy link
Collaborator

soumith commented Jun 22, 2017

@brunoalr i presume you didn't get time after that to work on this. I'm closing the PR for now, it's quite stale.

@brunoalr
Copy link
Author

@soumith sorry, it took me more time than expected to get access to a machine with a GPU. I tested the updated patch and it's working.

I opened a new PR: #1948
Please, take a look.

scotts added a commit to scotts/pytorch that referenced this pull request Mar 18, 2026
New commits included:

- 0c52fa6 Fix compilation of XPU part of kineto (pytorch#1292)
- f882254 Add MTIA_COUNTERS ActivityType and counter event output support (pytorch#1303)
- c6c84d0 Use whole data from PTI activity record (pytorch#1278)
- bb1e194 Add additionalLoggerCollector mechanism to ActivityProfilerController (pytorch#1290)
- 041c3e1 Disable test_record_function_fast (pytorch#1309)
- e8956c4 Integrate PyTorch's disabled tests mechanism into CI (pytorch#1311)
- 7d860f2 Fix unit test (pytorch#1305)
- 058386f Add Mac CPU workflow (pytorch#1304)
- c12ddc2 refactor CuptiCbidRegistry member function names (pytorch#1301)
- 3b5cdca Add comms Id to trace output JSON (pytorch#1300)

Authored with Claude.
pytorchmergebot pushed a commit that referenced this pull request Mar 19, 2026
New commits included:

- 0c52fa6 Fix compilation of XPU part of kineto (#1292)
- f882254 Add MTIA_COUNTERS ActivityType and counter event output support (#1303)
- c6c84d0 Use whole data from PTI activity record (#1278)
- bb1e194 Add additionalLoggerCollector mechanism to ActivityProfilerController (#1290)
- 041c3e1 Disable test_record_function_fast (#1309)
- e8956c4 Integrate PyTorch's disabled tests mechanism into CI (#1311)
- 7d860f2 Fix unit test (#1305)
- 058386f Add Mac CPU workflow (#1304)
- c12ddc2 refactor CuptiCbidRegistry member function names (#1301)
- 3b5cdca Add comms Id to trace output JSON (#1300)

Authored with Claude.
Pull Request resolved: #177753
Approved by: https://github.com/ryanzhang22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests fail on ppc64le

4 participants